Skip to content

Revert "CNDB-13457: Fix problems with byte-comparable versions in SAI" #1675

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Revert "CNDB-13457: Fix problems with byte-comparable versions in SAI" #1675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion build.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1723,7 +1723,6 @@
<jvmarg value="-Dcassandra.ring_delay_ms=1000"/>
<jvmarg value="-Dcassandra.tolerate_sstable_size=true"/>
<jvmarg value="-Dcassandra.sai.latest.version=aa"/>
<jvmarg value="-Dcassandra.trie_index_format_version=bb"/>
<jvmarg value="-Dcassandra.skip_sync=true" />
<jvmarg value="-Dlogback.configurationFile=file://${test.logback.configurationFile}"/>
</testmacrohelper>
Expand Down
4 changes: 2 additions & 2 deletions src/java/org/apache/cassandra/db/memtable/TrieMemtable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -729,11 +729,11 @@ long currentOperations()

private DecoratedKey firstPartitionKey(Direction direction)
{
Iterator<Map.Entry<ByteComparable.Preencoded, PartitionData>> iter = data.filteredEntryIterator(direction, PartitionData.class);
Iterator<Map.Entry<ByteComparable, PartitionData>> iter = data.filteredEntryIterator(direction, PartitionData.class);
if (!iter.hasNext())
return null;

Map.Entry<ByteComparable.Preencoded, PartitionData> entry = iter.next();
Map.Entry<ByteComparable, PartitionData> entry = iter.next();
return getPartitionKeyFromPath(metadata.get(), entry.getKey());
}

Expand Down
15 changes: 8 additions & 7 deletions src/java/org/apache/cassandra/db/memtable/TrieMemtableStage1.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ private static MemtablePartition createPartition(TableMetadata metadata, EnsureO
return new MemtablePartition(metadata, ensureOnHeap, key, data);
}

private static MemtablePartition getPartitionFromTrieEntry(TableMetadata metadata, EnsureOnHeap ensureOnHeap, Map.Entry<? extends ByteComparable, BTreePartitionData> en)
private static MemtablePartition getPartitionFromTrieEntry(TableMetadata metadata, EnsureOnHeap ensureOnHeap, Map.Entry<ByteComparable, BTreePartitionData> en)
{
DecoratedKey key = BufferDecoratedKey.fromByteComparable(en.getKey(),
BYTE_COMPARABLE_VERSION,
Expand All @@ -423,10 +423,11 @@ public FlushCollection<MemtablePartition> getFlushSet(PartitionPosition from, Pa
long keySize = 0;
int keyCount = 0;

for (Iterator<Map.Entry<ByteComparable.Preencoded, BTreePartitionData>> it = toFlush.entryIterator(); it.hasNext(); )
for (Iterator<Map.Entry<ByteComparable, BTreePartitionData>> it = toFlush.entryIterator(); it.hasNext(); )
{
Map.Entry<ByteComparable.Preencoded, BTreePartitionData> en = it.next();
byte[] keyBytes = DecoratedKey.keyFromByteComparable(en.getKey(), BYTE_COMPARABLE_VERSION, metadata().partitioner);
Map.Entry<ByteComparable, BTreePartitionData> en = it.next();
ByteComparable byteComparable = v -> en.getKey().asPeekableBytes(BYTE_COMPARABLE_VERSION);
byte[] keyBytes = DecoratedKey.keyFromByteComparable(byteComparable, BYTE_COMPARABLE_VERSION, metadata().partitioner);
keySize += keyBytes.length;
keyCount++;
}
Expand Down Expand Up @@ -628,11 +629,11 @@ long currentOperations()

private DecoratedKey firstPartitionKey(Direction direction)
{
Iterator<Map.Entry<ByteComparable.Preencoded, BTreePartitionData>> iter = data.entryIterator(direction);
Iterator<Map.Entry<ByteComparable, BTreePartitionData>> iter = data.entryIterator(direction);
if (!iter.hasNext())
return null;

Map.Entry<ByteComparable.Preencoded, BTreePartitionData> entry = iter.next();
Map.Entry<ByteComparable, BTreePartitionData> entry = iter.next();
return getPartitionKeyFromPath(metadata.get(), entry.getKey());
}

Expand All @@ -652,7 +653,7 @@ static class MemtableUnfilteredPartitionIterator extends AbstractUnfilteredParti
private final TableMetadata metadata;
private final EnsureOnHeap ensureOnHeap;
private final Trie<BTreePartitionData> source;
private final Iterator<Map.Entry<ByteComparable.Preencoded, BTreePartitionData>> iter;
private final Iterator<Map.Entry<ByteComparable, BTreePartitionData>> iter;
private final ColumnFilter columnFilter;
private final DataRange dataRange;

Expand Down
5 changes: 0 additions & 5 deletions src/java/org/apache/cassandra/db/tries/InMemoryReadTrie.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1132,11 +1132,6 @@ public boolean isEmpty()
return isNull(root);
}

public ByteComparable.Version byteComparableVersion()
{
return byteComparableVersion;
}

/**
* Override of dump to provide more detailed printout that includes the type of each node in the trie.
* We do this via a wrapping cursor that returns a content string for the type of node for every node we return.
Expand Down
22 changes: 11 additions & 11 deletions src/java/org/apache/cassandra/db/tries/Trie.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,15 +375,15 @@ public void forEachValue(ValueConsumer<T> consumer)
/**
* Call the given consumer on all (path, content) pairs with non-null content in the trie in order.
*/
public void forEachEntry(BiConsumer<ByteComparable.Preencoded, T> consumer)
public void forEachEntry(BiConsumer<ByteComparable, T> consumer)
{
forEachEntry(Direction.FORWARD, consumer);
}

/**
* Call the given consumer on all (path, content) pairs with non-null content in the trie in order.
*/
public void forEachEntry(Direction direction, BiConsumer<ByteComparable.Preencoded, T> consumer)
public void forEachEntry(Direction direction, BiConsumer<ByteComparable, T> consumer)
{
Cursor<T> cursor = cursor(direction);
process(new TrieEntriesWalker.WithConsumer<T>(consumer, cursor.byteComparableVersion()), cursor);
Expand Down Expand Up @@ -427,7 +427,7 @@ public Void forEachValueSkippingBranches(Direction direction, ValueConsumer<T> c
* Call the given consumer on all (path, content) pairs with non-null content in the trie in order, skipping all
* branches below the top content-bearing node.
*/
public void forEachEntrySkippingBranches(Direction direction, BiConsumer<ByteComparable.Preencoded, T> consumer)
public void forEachEntrySkippingBranches(Direction direction, BiConsumer<ByteComparable, T> consumer)
{
Cursor<T> cursor = cursor(direction);
processSkippingBranches(new TrieEntriesWalker.WithConsumer<T>(consumer, cursor.byteComparableVersion()), cursor);
Expand Down Expand Up @@ -545,55 +545,55 @@ public Trie<T> subtrie(ByteComparable left, ByteComparable right)
/**
* Returns the ordered entry set of this trie's content as an iterable.
*/
public Iterable<Map.Entry<ByteComparable.Preencoded, T>> entrySet()
public Iterable<Map.Entry<ByteComparable, T>> entrySet()
{
return this::entryIterator;
}

/**
* Returns the ordered entry set of this trie's content as an iterable.
*/
public Iterable<Map.Entry<ByteComparable.Preencoded, T>> entrySet(Direction direction)
public Iterable<Map.Entry<ByteComparable, T>> entrySet(Direction direction)
{
return () -> entryIterator(direction);
}

/**
* Returns the ordered entry set of this trie's content in an iterator.
*/
public Iterator<Map.Entry<ByteComparable.Preencoded, T>> entryIterator()
public Iterator<Map.Entry<ByteComparable, T>> entryIterator()
{
return entryIterator(Direction.FORWARD);
}

/**
* Returns the ordered entry set of this trie's content in an iterator.
*/
public Iterator<Map.Entry<ByteComparable.Preencoded, T>> entryIterator(Direction direction)
public Iterator<Map.Entry<ByteComparable, T>> entryIterator(Direction direction)
{
return new TrieEntriesIterator.AsEntries<>(cursor(direction));
}

/**
* Returns the ordered entry set of this trie's content in an iterable, filtered by the given type.
*/
public <U extends T> Iterable<Map.Entry<ByteComparable.Preencoded, U>> filteredEntrySet(Class<U> clazz)
public <U extends T> Iterable<Map.Entry<ByteComparable, U>> filteredEntrySet(Class<U> clazz)
{
return filteredEntrySet(Direction.FORWARD, clazz);
}

/**
* Returns the ordered entry set of this trie's content in an iterable, filtered by the given type.
*/
public <U extends T> Iterable<Map.Entry<ByteComparable.Preencoded, U>> filteredEntrySet(Direction direction, Class<U> clazz)
public <U extends T> Iterable<Map.Entry<ByteComparable, U>> filteredEntrySet(Direction direction, Class<U> clazz)
{
return () -> filteredEntryIterator(direction, clazz);
}

/**
* Returns the ordered entry set of this trie's content in an iterator, filtered by the given type.
*/
public <U extends T> Iterator<Map.Entry<ByteComparable.Preencoded, U>> filteredEntryIterator(Direction direction, Class<U> clazz)
public <U extends T> Iterator<Map.Entry<ByteComparable, U>> filteredEntryIterator(Direction direction, Class<U> clazz)
{
return new TrieEntriesIterator.AsEntriesFilteredByType<>(cursor(direction), clazz);
}
Expand Down Expand Up @@ -801,7 +801,7 @@ public Trie<T> prefixedBy(ByteComparable prefix)
* Returns an entry set containing all tail tree constructed at the points that contain content of
* the given type.
*/
public Iterable<Map.Entry<ByteComparable.Preencoded, Trie<T>>> tailTries(Direction direction, Class<? extends T> clazz)
public Iterable<Map.Entry<ByteComparable, Trie<T>>> tailTries(Direction direction, Class<? extends T> clazz)
{
return () -> new TrieTailsIterator.AsEntries<>(cursor(direction), clazz);
}
Expand Down
10 changes: 5 additions & 5 deletions src/java/org/apache/cassandra/db/tries/TrieEntriesIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,15 @@ ByteComparable.Version byteComparableVersion()
/**
* Iterator representing the content of the trie a sequence of (path, content) pairs.
*/
static class AsEntries<T> extends TrieEntriesIterator<T, Map.Entry<ByteComparable.Preencoded, T>>
static class AsEntries<T> extends TrieEntriesIterator<T, Map.Entry<ByteComparable, T>>
{
public AsEntries(Trie.Cursor<T> cursor)
{
super(cursor, Predicates.alwaysTrue());
}

@Override
protected Map.Entry<ByteComparable.Preencoded, T> mapContent(T content, byte[] bytes, int byteLength)
protected Map.Entry<ByteComparable, T> mapContent(T content, byte[] bytes, int byteLength)
{
return toEntry(byteComparableVersion(), content, bytes, byteLength);
}
Expand All @@ -103,7 +103,7 @@ protected Map.Entry<ByteComparable.Preencoded, T> mapContent(T content, byte[] b
/**
* Iterator representing the content of the trie a sequence of (path, content) pairs.
*/
static class AsEntriesFilteredByType<T, U extends T> extends TrieEntriesIterator<T, Map.Entry<ByteComparable.Preencoded, U>>
static class AsEntriesFilteredByType<T, U extends T> extends TrieEntriesIterator<T, Map.Entry<ByteComparable, U>>
{
public AsEntriesFilteredByType(Trie.Cursor<T> cursor, Class<U> clazz)
{
Expand All @@ -112,13 +112,13 @@ public AsEntriesFilteredByType(Trie.Cursor<T> cursor, Class<U> clazz)

@Override
@SuppressWarnings("unchecked") // checked by the predicate
protected Map.Entry<ByteComparable.Preencoded, U> mapContent(T content, byte[] bytes, int byteLength)
protected Map.Entry<ByteComparable, U> mapContent(T content, byte[] bytes, int byteLength)
{
return toEntry(byteComparableVersion(), (U) content, bytes, byteLength);
}
}

static <T> java.util.Map.Entry<ByteComparable.Preencoded, T> toEntry(ByteComparable.Version version, T content, byte[] bytes, int byteLength)
static <T> java.util.Map.Entry<ByteComparable, T> toEntry(ByteComparable.Version version, T content, byte[] bytes, int byteLength)
{
return new AbstractMap.SimpleImmutableEntry<>(toByteComparable(version, bytes, byteLength), content);
}
Expand Down
4 changes: 2 additions & 2 deletions src/java/org/apache/cassandra/db/tries/TrieEntriesWalker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ public void content(T content)
*/
static class WithConsumer<T> extends TrieEntriesWalker<T, Void>
{
private final BiConsumer<ByteComparable.Preencoded, T> consumer;
private final BiConsumer<ByteComparable, T> consumer;
private final ByteComparable.Version byteComparableVersion;

public WithConsumer(BiConsumer<ByteComparable.Preencoded, T> consumer, ByteComparable.Version byteComparableVersion)
public WithConsumer(BiConsumer<ByteComparable, T> consumer, ByteComparable.Version byteComparableVersion)
{
this.consumer = consumer;
this.byteComparableVersion = byteComparableVersion;
Expand Down
2 changes: 1 addition & 1 deletion src/java/org/apache/cassandra/db/tries/TriePathReconstructor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public void resetPathLength(int newLength)
keyPos = newLength;
}

static ByteComparable.Preencoded toByteComparable(ByteComparable.Version byteComparableVersion, byte[] bytes, int byteLength)
static ByteComparable toByteComparable(ByteComparable.Version byteComparableVersion, byte[] bytes, int byteLength)
{
// Taking a copy here to make sure it does not get modified when the cursor advances.
return ByteComparable.preencoded(byteComparableVersion, Arrays.copyOf(bytes, byteLength));
Expand Down
6 changes: 3 additions & 3 deletions src/java/org/apache/cassandra/db/tries/TrieTailsIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,17 @@ ByteComparable.Version byteComparableVersion()
* {@code tail} is the branch of the trie rooted at the selected content node (reachable by following
* {@code path}). The tail trie will have the selected content at its root.
*/
static class AsEntries<T> extends TrieTailsIterator<T, Map.Entry<ByteComparable.Preencoded, Trie<T>>>
static class AsEntries<T> extends TrieTailsIterator<T, Map.Entry<ByteComparable, Trie<T>>>
{
public AsEntries(Trie.Cursor<T> cursor, Class<? extends T> clazz)
{
super(cursor, clazz::isInstance);
}

@Override
protected Map.Entry<ByteComparable.Preencoded, Trie<T>> mapContent(T value, Trie<T> tailTrie, byte[] bytes, int byteLength)
protected Map.Entry<ByteComparable, Trie<T>> mapContent(T value, Trie<T> tailTrie, byte[] bytes, int byteLength)
{
ByteComparable.Preencoded key = toByteComparable(byteComparableVersion(), bytes, byteLength);
ByteComparable key = toByteComparable(byteComparableVersion(), bytes, byteLength);
return new AbstractMap.SimpleImmutableEntry<>(key, tailTrie);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/java/org/apache/cassandra/index/sai/disk/MemtableTermsIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,16 @@ public class MemtableTermsIterator implements TermsIterator
{
private final ByteBuffer minTerm;
private final ByteBuffer maxTerm;
private final Iterator<Pair<ByteComparable.Preencoded, List<RowMapping.RowIdWithFrequency>>> iterator;
private final Iterator<Pair<ByteComparable, List<RowMapping.RowIdWithFrequency>>> iterator;

private Pair<ByteComparable.Preencoded, List<RowMapping.RowIdWithFrequency>> current;
private Pair<ByteComparable, List<RowMapping.RowIdWithFrequency>> current;

private int maxSSTableRowId = -1;
private int minSSTableRowId = Integer.MAX_VALUE;

public MemtableTermsIterator(ByteBuffer minTerm,
ByteBuffer maxTerm,
Iterator<Pair<ByteComparable.Preencoded, List<RowMapping.RowIdWithFrequency>>> iterator)
Iterator<Pair<ByteComparable, List<RowMapping.RowIdWithFrequency>>> iterator)
{
Preconditions.checkArgument(iterator != null);
this.minTerm = minTerm;
Expand Down
4 changes: 2 additions & 2 deletions src/java/org/apache/cassandra/index/sai/disk/v1/MemtableIndexWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,14 @@ private long flush(DecoratedKey minKey, DecoratedKey maxKey, AbstractType<?> ter
var docLengths = new Int2IntHashMap(Integer.MIN_VALUE);
Arrays.stream(((TrieMemtableIndex) memtableIndex).getRangeIndexes())
.map(TrieMemoryIndex.class::cast)
.forEach(trieMemoryIndex ->
.forEach(trieMemoryIndex ->
trieMemoryIndex.getDocLengths().forEach((pk, length) -> {
int rowId = rowMapping.get(pk);
if (rowId >= 0)
docLengths.put(rowId, (int) length);
})
);

indexMetas = writer.writeAll(metadataBuilder.intercept(terms), docLengths);
numRows = writer.getPostingsCount();
}
Expand Down
9 changes: 3 additions & 6 deletions src/java/org/apache/cassandra/index/sai/disk/v1/SegmentMetadataBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import org.apache.cassandra.index.sai.IndexContext;
import org.apache.cassandra.index.sai.disk.PostingList;
import org.apache.cassandra.index.sai.disk.TermsIterator;
import org.apache.cassandra.index.sai.disk.format.IndexComponentType;
import org.apache.cassandra.index.sai.disk.format.IndexComponents;
import org.apache.cassandra.index.sai.disk.format.Version;
import org.apache.cassandra.index.sai.disk.v1.kdtree.MutableOneDimPointValues;
Expand All @@ -58,7 +57,6 @@ public class SegmentMetadataBuilder
private final long segmentRowIdOffset;

private final List<Closeable> interceptors = new ArrayList<>();
private final ByteComparable.Version byteComparableVersion;

private boolean built = false;

Expand All @@ -81,11 +79,10 @@ public SegmentMetadataBuilder(long segmentRowIdOffset, IndexComponents.ForWrite
{
IndexContext context = Objects.requireNonNull(components.context());
this.segmentRowIdOffset = segmentRowIdOffset;
this.byteComparableVersion = components.byteComparableVersionFor(IndexComponentType.TERMS_DATA);

int histogramSize = context.getIntOption(HISTOGRAM_SIZE_OPTION, 128);
int mostFrequentTermsCount = context.getIntOption(MFT_COUNT_OPTION, 128);
this.termsDistributionBuilder = new TermsDistribution.Builder(context.getValidator(), byteComparableVersion, histogramSize, mostFrequentTermsCount);
this.termsDistributionBuilder = new TermsDistribution.Builder(context.getValidator(), histogramSize, mostFrequentTermsCount);
}

public void setKeyRange(@Nonnull PrimaryKey minKey, @Nonnull PrimaryKey maxKey)
Expand Down Expand Up @@ -350,7 +347,7 @@ public void intersect(IntersectVisitor visitor) throws IOException
if (!Arrays.equals(term, lastTerm))
{
if (lastTerm != null)
builder.add(ByteComparable.preencoded(builder.byteComparableVersion, lastTerm), count);
builder.add(ByteComparable.preencoded(TypeUtil.BYTE_COMPARABLE_VERSION, lastTerm), count);


count = 0;
Expand All @@ -366,7 +363,7 @@ public void close() throws IOException
{
if (lastTerm != null)
{
builder.add(ByteComparable.preencoded(builder.byteComparableVersion, lastTerm), count);
builder.add(ByteComparable.preencoded(TypeUtil.BYTE_COMPARABLE_VERSION, lastTerm), count);
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/java/org/apache/cassandra/index/sai/disk/v1/kdtree/ImmutableOneDimPointValues.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.apache.cassandra.index.sai.disk.TermsIterator;
import org.apache.cassandra.index.sai.disk.oldlucene.MutablePointValues;
import org.apache.cassandra.index.sai.utils.TypeUtil;
import org.apache.cassandra.utils.bytecomparable.ByteComparable;
import org.apache.cassandra.utils.bytecomparable.ByteSourceInverse;
import org.apache.lucene.util.bkd.BKDWriter;

Expand Down Expand Up @@ -56,7 +55,7 @@ public void intersect(IntersectVisitor visitor) throws IOException
{
while (termEnum.hasNext())
{
ByteSourceInverse.readBytesMustFit(((ByteComparable.Preencoded) termEnum.next()).getPreencodedBytes(),
ByteSourceInverse.readBytesMustFit(termEnum.next().asComparableBytes(TypeUtil.BYTE_COMPARABLE_VERSION),
scratch);

try (final PostingList postings = termEnum.postings())
Expand Down
Loading
Loading