Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CNDB-12407: lazily load token in PrimaryKeyWithSource #1500

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Prev Previous commit
Next Next commit
Temp fix to workaround SegmentMetadata inconsistencies
The min/max partition key is not always a pointer
to the segment metadata's min/max sstable row id.
I thought it was. Removing that assertion.
michaeljmarshall committed Jan 17, 2025
commit 0955f4a1ce5de2400a4b5d1b089607b1b2421770
Original file line number Diff line number Diff line change
@@ -148,31 +148,21 @@ private SegmentMetadata(IndexInput input, IndexContext context, Version version,
this.minSSTableRowId = input.readLong();
this.maxSSTableRowId = input.readLong();

// Read the real min and max partition from the segment metadata.
DecoratedKey minPartition = DatabaseDescriptor.getPartitioner().decorateKey(readBytes(input));
DecoratedKey maxPartition = DatabaseDescriptor.getPartitioner().decorateKey(readBytes(input));
// The next values are the min/max partition keys. As a tempory test, we are skipping them because they are
// not as useful as the min/max row ids, which are always correct for both flushed and compacted sstables.
skipBytes(input);
skipBytes(input);

// Get the fully qualified PrimaryKey min and max objects to ensure that we skip several edge cases related
// to possibly confusing equality semantics where PrimaryKeyWithSource slightly diverges from PrimaryKey where
// PrimaryKey is just a partition key without a materializable clustering key.
PrimaryKey min, max;
if (sstableContext.sstable.metadata().comparator.size() > 0)
final PrimaryKey min, max;
try (var pkm = sstableContext.primaryKeyMapFactory().newPerSSTablePrimaryKeyMap())
{
try (var pkm = sstableContext.primaryKeyMapFactory().newPerSSTablePrimaryKeyMap())
{
// We need to load eagerly to allow us to close the partition key map. Otherwise, all tests will
// pass due to the side effect of calling partitionKey(), but it'll fail when you remove the -ea flag.
min = pkm.primaryKeyFromRowId(minSSTableRowId).loadDeferred();
max = pkm.primaryKeyFromRowId(maxSSTableRowId).loadDeferred();
assert min.partitionKey().equals(minPartition) : String.format("Min partition key mismatch: %s != %s", min, minPartition);
assert max.partitionKey().equals(maxPartition) : String.format("Max partition key mismatch: %s != %s", max, maxPartition);
}
}
else
{
var primaryKeyFactory = context.keyFactory();
min = primaryKeyFactory.createPartitionKeyOnly(minPartition);
max = primaryKeyFactory.createPartitionKeyOnly(maxPartition);
// We need to load eagerly to allow us to close the partition key map. Otherwise, all tests will
// pass due to the side effect of calling partitionKey(), but it'll fail when you remove the -ea flag.
min = pkm.primaryKeyFromRowId(minSSTableRowId).loadDeferred();
max = pkm.primaryKeyFromRowId(maxSSTableRowId).loadDeferred();
}

this.minKey = new PrimaryKeyWithSource(min, sstableContext.sstable.getId(), minSSTableRowId, min, max);
@@ -332,6 +322,12 @@ private static ByteBuffer readBytes(IndexInput input) throws IOException
return ByteBuffer.wrap(bytes);
}

private static void skipBytes(IndexInput input) throws IOException
{
int len = input.readVInt();
input.skipBytes(len);
}

static void writeBytes(ByteBuffer buf, IndexOutput out)
{
try