You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "adelapena (via GitHub)" <gi...@apache.org> on 2023/06/12 16:08:42 UTC
[GitHub] [cassandra] adelapena commented on a diff in pull request #2409: Numeric on-disk index write and search
adelapena commented on code in PR #2409:
URL: https://github.com/apache/cassandra/pull/2409#discussion_r1226885368
##########
src/java/org/apache/cassandra/index/sai/disk/v1/kdtree/BKDWriter.java:
##########
@@ -0,0 +1,782 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.index.sai.disk.v1.kdtree;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.IntFunction;
+
+import com.google.common.base.MoreObjects;
+
+import org.apache.cassandra.index.sai.disk.io.RAMIndexOutput;
+import org.apache.cassandra.index.sai.disk.v1.SAICodecUtils;
+import org.apache.lucene.codecs.MutablePointValues;
+import org.apache.lucene.store.DataOutput;
+import org.apache.lucene.store.GrowableByteArrayDataOutput;
+import org.apache.lucene.store.IndexOutput;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.FutureArrays;
+import org.apache.lucene.util.IntroSorter;
+import org.apache.lucene.util.LongBitSet;
+import org.apache.lucene.util.Sorter;
+import org.apache.lucene.util.bkd.MutablePointsReaderUtils;
+
+/**
+ * This is a specialisation of the lucene BKDWriter that only writes a single dimension.
+ * <p>
+ * Recursively builds a block KD-tree to assign all incoming points in N-dim space to smaller
+ * and smaller N-dim rectangles (cells) until the number of points in a given
+ * rectangle is <= <code>maxPointsInLeafNode</code>. The tree is
+ * fully balanced, which means the leaf nodes will have between 50% and 100% of
+ * the requested <code>maxPointsInLeafNode</code>. Values that fall exactly
+ * on a cell boundary may be in either cell.
+ *
+ * <p>
+ * See <a href="https://www.cs.duke.edu/~pankaj/publications/papers/bkd-sstd.pdf">this paper</a> for details.
+ *
+ * <p>This consumes heap during writing: it allocates a <code>LongBitSet(numPoints)</code>,
+ * and then uses up to the specified {@code maxMBSortInHeap} heap space for writing.
+ *
+ * <p>
+ * <b>NOTE</b>: This can write at most Integer.MAX_VALUE * <code>maxPointsInLeafNode</code> total points.
+ *
+ * lucene.experimental
+ */
+public class BKDWriter
+{
+ // Enable to check that values are added to the tree in correct order and within bounds
+ private static final boolean DEBUG = false;
+
+ // Default maximum number of point in each leaf block
+ public static final int DEFAULT_MAX_POINTS_IN_LEAF_NODE = 1024;
+
+ private final int bytesPerValue;
+ private final BytesRef scratchBytesRef1 = new BytesRef();
+ private final LongBitSet docsSeen;
+ private final int maxPointsInLeafNode;
+ private final byte[] minPackedValue;
+ private final byte[] maxPackedValue;
+ private long pointCount;
+ private final long maxDoc;
+
+ public BKDWriter(long maxDoc, int bytesPerValue, int maxPointsInLeafNode)
+ {
+ if (maxPointsInLeafNode <= 0)
+ throw new IllegalArgumentException("maxPointsInLeafNode must be > 0; got " + maxPointsInLeafNode);
+ if (maxPointsInLeafNode > ArrayUtil.MAX_ARRAY_LENGTH)
+ throw new IllegalArgumentException("maxPointsInLeafNode must be <= ArrayUtil.MAX_ARRAY_LENGTH (= " +
+ ArrayUtil.MAX_ARRAY_LENGTH + "); got " + maxPointsInLeafNode);
+
+ this.maxPointsInLeafNode = maxPointsInLeafNode;
+ this.bytesPerValue = bytesPerValue;
+ this.maxDoc = maxDoc;
+ docsSeen = new LongBitSet(maxDoc);
+
+ minPackedValue = new byte[bytesPerValue];
+ maxPackedValue = new byte[bytesPerValue];
+ }
+
+ public long getPointCount()
+ {
+ return pointCount;
+ }
+
+ public int getBytesPerValue()
+ {
+ return bytesPerValue;
+ }
+
+ public int getMaxPointsInLeafNode()
+ {
+ return maxPointsInLeafNode;
+ }
+
+ /**
+ * Write a field from a {@link MutablePointValues}. This way of writing
+ * points is faster than regular writes with BKDWriter#add since
Review Comment:
What method is `BKDWriter#add`?
##########
src/java/org/apache/cassandra/index/sai/disk/v1/kdtree/BKDReader.java:
##########
@@ -0,0 +1,470 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.cassandra.index.sai.disk.v1.kdtree;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Comparator;
+import java.util.PriorityQueue;
+import java.util.concurrent.TimeUnit;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Stopwatch;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.cassandra.exceptions.QueryCancelledException;
+import org.apache.cassandra.index.sai.IndexContext;
+import org.apache.cassandra.index.sai.QueryContext;
+import org.apache.cassandra.index.sai.disk.io.IndexFileUtils;
+import org.apache.cassandra.index.sai.disk.io.SeekingRandomAccessInput;
+import org.apache.cassandra.index.sai.disk.v1.DirectReaders;
+import org.apache.cassandra.index.sai.disk.v1.postings.FilteringPostingList;
+import org.apache.cassandra.index.sai.disk.v1.postings.MergePostingList;
+import org.apache.cassandra.index.sai.disk.v1.postings.PostingsReader;
+import org.apache.cassandra.index.sai.metrics.QueryEventListener;
+import org.apache.cassandra.index.sai.postings.PeekablePostingList;
+import org.apache.cassandra.index.sai.postings.PostingList;
+import org.apache.cassandra.io.util.FileHandle;
+import org.apache.cassandra.io.util.FileUtils;
+import org.apache.cassandra.utils.Throwables;
+import org.apache.lucene.index.CorruptIndexException;
+import org.apache.lucene.index.PointValues.Relation;
+import org.apache.lucene.store.IndexInput;
+import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.FixedBitSet;
+import org.apache.lucene.util.FutureArrays;
+import org.apache.lucene.util.packed.DirectWriter;
+
+/**
+ * Handles intersection of a multidimensional shape in byte[] space with a block KD-tree previously written with
+ * {@link BKDWriter}.
Review Comment:
Isn't `BKDWriter` unidimensional?
##########
src/java/org/apache/cassandra/index/sai/disk/v1/kdtree/BKDWriter.java:
##########
@@ -0,0 +1,782 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.index.sai.disk.v1.kdtree;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.IntFunction;
+
+import com.google.common.base.MoreObjects;
+
+import org.apache.cassandra.index.sai.disk.io.RAMIndexOutput;
+import org.apache.cassandra.index.sai.disk.v1.SAICodecUtils;
+import org.apache.lucene.codecs.MutablePointValues;
+import org.apache.lucene.store.DataOutput;
+import org.apache.lucene.store.GrowableByteArrayDataOutput;
+import org.apache.lucene.store.IndexOutput;
+import org.apache.lucene.util.ArrayUtil;
+import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.FutureArrays;
+import org.apache.lucene.util.IntroSorter;
+import org.apache.lucene.util.LongBitSet;
+import org.apache.lucene.util.Sorter;
+import org.apache.lucene.util.bkd.MutablePointsReaderUtils;
+
+/**
+ * This is a specialisation of the lucene BKDWriter that only writes a single dimension.
+ * <p>
+ * Recursively builds a block KD-tree to assign all incoming points in N-dim space to smaller
+ * and smaller N-dim rectangles (cells) until the number of points in a given
Review Comment:
These two lines seem to contradict each other on the number of dimensions. Maybe the second line could start with something like `Lucene's implementation recursively builds...`
##########
src/java/org/apache/cassandra/index/sai/disk/io/RAMIndexOutput.java:
##########
@@ -70,11 +75,16 @@ public void writeTo(IndexOutput externalOut) throws IOException
externalOut.writeBytes(out.getBytes(), 0, out.getPosition());
}
- public BytesRef getBytes()
+ public BytesRef getBytesRef()
{
return new BytesRef(out.getBytes(), 0, out.getPosition());
}
+ public void writeTo(byte[] bytes, int offset)
Review Comment:
Nit: the offset argument passed by the callers is always zero, so perhaps this could be simplified to `writeTo(byte[] bytes)`. Also, I would place the method right after the other `writeTo(IndexOutput)` method, so similar methods stay together.
##########
src/java/org/apache/cassandra/index/sai/utils/TypeUtil.java:
##########
@@ -130,6 +131,23 @@ public static ByteComparable max(ByteComparable a, ByteComparable b)
return a == null ? b : (b == null || ByteComparable.compare(b, a, ByteComparable.Version.OSS50) < 0) ? a : b;
}
+ /**
+ * Returns the value length for the given {@link AbstractType}, selecting 16 for types
+ * that officially use VARIABLE_LENGTH but are, in fact, of a fixed length.
+ */
+ public static int fixedSizeOf(AbstractType<?> type)
+ {
+ if (type.isValueLengthFixed())
+ return type.valueLengthIfFixed();
+ else if (isInetAddress(type))
+ return 16;
+ else if (isBigInteger(type))
+ return 20;
+ else if (type instanceof DecimalType)
Review Comment:
I think this can use `isBigDecimal(type)`.
##########
src/java/org/apache/cassandra/index/sai/disk/v1/segment/SegmentBuilder.java:
##########
@@ -71,10 +73,48 @@
private ByteBuffer minTerm;
private ByteBuffer maxTerm;
+ final AbstractType<?> termComparator;
long totalBytesAllocated;
int rowCount = 0;
int maxSegmentRowId = -1;
+ public static class KDTreeSegmentBuilder extends SegmentBuilder
+ {
+ protected final byte[] buffer;
+ private final BKDTreeRamBuffer kdTreeRamBuffer;
+
+ public KDTreeSegmentBuilder(AbstractType<?> termComparator, NamedMemoryLimiter limiter)
+ {
+ super(termComparator, limiter);
+
+ int typeSize = TypeUtil.fixedSizeOf(termComparator);
+ this.kdTreeRamBuffer = new BKDTreeRamBuffer(typeSize);
+ this.buffer = new byte[typeSize];
+ this.totalBytesAllocated = this.kdTreeRamBuffer.ramBytesUsed();
+ }
+
+ public boolean isEmpty()
+ {
+ return kdTreeRamBuffer.numRows() == 0;
+ }
+
+ protected long addInternal(ByteBuffer term, int segmentRowId)
Review Comment:
Nit: add `@Override`
##########
src/java/org/apache/cassandra/index/sai/disk/v1/kdtree/OneDimBKDPostingsWriter.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.cassandra.index.sai.disk.v1.kdtree;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.PriorityQueue;
+import java.util.TreeMap;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
+
+import com.google.common.base.Stopwatch;
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Multimap;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.agrona.collections.IntArrayList;
+import org.apache.cassandra.config.CassandraRelevantProperties;
+import org.apache.cassandra.index.sai.IndexContext;
+import org.apache.cassandra.index.sai.disk.v1.postings.MergePostingList;
+import org.apache.cassandra.index.sai.disk.v1.postings.PackedLongsPostingList;
+import org.apache.cassandra.index.sai.disk.v1.postings.PostingsWriter;
+import org.apache.cassandra.index.sai.postings.PeekablePostingList;
+import org.apache.cassandra.index.sai.postings.PostingList;
+import org.apache.cassandra.utils.FBUtilities;
+import org.apache.lucene.store.IndexOutput;
+import org.apache.lucene.util.packed.PackedLongValues;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+
+/**
+ * Writes auxiliary posting lists for bkd tree nodes. If a node has a posting list attached, it will contain every row
+ * id
+ * from all leaves reachable from that node.
+ *
+ * Writer is stateful, because it needs to collect data from bkd index data structure first to find set of eligible
+ * nodes and leaf nodes reachable from them.
+ *
+ * This is an optimised writer for 1-dim points, where we know that leaf blocks are written in value order (in this
+ * order we pass them to the {@link BKDWriter}). That allows us to skip reading the leaves, instead just order leaf
+ * blocks by their offset in the index file, and correlate them with buffered posting lists. We can't make this
+ * assumption for multi-dim case.
+ */
+public class OneDimBKDPostingsWriter implements TraversingBKDReader.IndexTreeTraversalCallback
Review Comment:
The package is named kdtree, most of the classes are named BKD*, and this class is named OneDimBKD*. However, I think everything is about a single unidimensional bkd tree implementation. Is that right? If it is, probably we should use the same naming for the package and all the classes. OneDimBKD* seems the clearest name, but I wouldn't oppose to BKD* or even KD* for brevity.
##########
src/java/org/apache/cassandra/index/sai/disk/v1/segment/SegmentBuilder.java:
##########
@@ -71,10 +73,48 @@
private ByteBuffer minTerm;
private ByteBuffer maxTerm;
+ final AbstractType<?> termComparator;
long totalBytesAllocated;
int rowCount = 0;
int maxSegmentRowId = -1;
+ public static class KDTreeSegmentBuilder extends SegmentBuilder
+ {
+ protected final byte[] buffer;
+ private final BKDTreeRamBuffer kdTreeRamBuffer;
+
+ public KDTreeSegmentBuilder(AbstractType<?> termComparator, NamedMemoryLimiter limiter)
+ {
+ super(termComparator, limiter);
+
+ int typeSize = TypeUtil.fixedSizeOf(termComparator);
+ this.kdTreeRamBuffer = new BKDTreeRamBuffer(typeSize);
+ this.buffer = new byte[typeSize];
+ this.totalBytesAllocated = this.kdTreeRamBuffer.ramBytesUsed();
+ }
+
+ public boolean isEmpty()
Review Comment:
Nit: add `@Override`
##########
src/java/org/apache/cassandra/index/sai/disk/v1/kdtree/ImmutableOneDimPointValues.java:
##########
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.cassandra.index.sai.disk.v1.kdtree;
+
+import java.io.IOException;
+
+import org.apache.cassandra.db.marshal.AbstractType;
+import org.apache.cassandra.index.sai.postings.PostingList;
+import org.apache.cassandra.index.sai.utils.TermsIterator;
+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.codecs.MutablePointValues;
+import org.apache.lucene.util.bkd.BKDWriter;
+
+/**
+ * {@link MutablePointValues} that prevents buffered points from reordering, and always skips sorting phase in Lucene
Review Comment:
```suggestion
* {@link MutableOneDimPointValues} that prevents buffered points from reordering, and always skips sorting phase in Lucene
```
##########
src/java/org/apache/cassandra/index/sai/disk/v1/SSTableIndexWriter.java:
##########
@@ -327,7 +327,9 @@ private void writeSegmentsMetadata() throws IOException
private SegmentBuilder newSegmentBuilder()
{
- SegmentBuilder builder = new SegmentBuilder.RAMStringSegmentBuilder(indexContext.getValidator(), limiter);
+ SegmentBuilder builder = TypeUtil.isLiteral(indexContext.getValidator())
Review Comment:
Not directly related to the changes, but I think that the method `TypeUtil.isUTF8OrAscii` invoked by `TypeUtil.isLiteral` could use `type instanceof StringType` instead of `type instanceof StringType || type instanceof AsciiType`.
##########
src/java/org/apache/cassandra/index/sai/disk/format/IndexComponent.java:
##########
@@ -31,6 +31,12 @@
*/
META("Meta"),
+ /**
+ * KDTree written by {@code BKDWriter} indexes mappings of term to one or more segment row IDs
+ * (segment row ID = SSTable row ID - segment row ID offset).
+ */
+ KD_TREE("KDTree"),
Review Comment:
Should it be `BKDTree`?
##########
src/java/org/apache/cassandra/index/sai/utils/TypeUtil.java:
##########
@@ -130,6 +131,23 @@ public static ByteComparable max(ByteComparable a, ByteComparable b)
return a == null ? b : (b == null || ByteComparable.compare(b, a, ByteComparable.Version.OSS50) < 0) ? a : b;
}
+ /**
+ * Returns the value length for the given {@link AbstractType}, selecting 16 for types
+ * that officially use VARIABLE_LENGTH but are, in fact, of a fixed length.
+ */
+ public static int fixedSizeOf(AbstractType<?> type)
Review Comment:
Not directly related to this ticket, but it seems to me that the methods on `TypeUtil` do a lot of repeated work in the hot path just to determine the data type of the caller. These operations include finding the base type of reversed multiple times, multiple `instanceof` calls, etc. All these operations on each index are called with the same `IndexContext#getValidator` argument, needlessly repeating work for every column value.
I understand that most of the methods on `TypeUtil` are things that would normally be part of `AbstractType`, so each particular data type can provide its own implementation. But we don't want to couple the generic data types with SAI, so we have this class instead.
I think that we could add a kind of `TermType` class decorating `AbstractType`, and subclass it for every data type that gets special treatment (`LONG`, `VARINT`, `DECIMAL` and `INET`). Then, `IndexContext` would hold the adequate instance of `TermType`. This instance would be able to provide the same operations as the current `TypeUtil`, but without the type checks.
If that makes sense, we could do it on a separate ticket.
##########
src/java/org/apache/cassandra/index/sai/utils/TypeUtil.java:
##########
@@ -186,9 +204,38 @@ public static ByteSource asComparableBytes(ByteBuffer value, AbstractType<?> typ
{
if (type instanceof InetAddressType || type instanceof IntegerType || type instanceof DecimalType)
return ByteSource.optionalFixedLength(ByteBufferAccessor.instance, value);
+ // The LongType.asComparableBytes uses variableLengthInteger which doesn't play well with
+ // the KDTree because it is expecting fixed length data. So for SAI we use a optionalSignedFixedLengthNumber
+ // to keep all comparable values the same length
+ else if (type instanceof LongType)
+ return ByteSource.optionalSignedFixedLengthNumber(ByteBufferAccessor.instance, value);
return type.asComparableBytes(value, version);
}
+ /**
+ * Fills a byte array with the comparable bytes for a type.
+ * <p>
+ * This method expects a {@code value} parameter generated by calling {@link #asIndexBytes(ByteBuffer, AbstractType)}.
+ * It is not generally safe to pass the output of other serialization methods to this method. For instance, it is
+ * not generally safe to pass the output of {@link AbstractType#decompose(Object)} as the {@code value} parameter
+ * (there are certain types for which this is technically OK, but that doesn't hold for all types).
+ *
+ * @param value a value buffer returned by {@link #asIndexBytes(ByteBuffer, AbstractType)}
+ * @param type the type associated with the encoded {@code value} parameter
+ * @param bytes this method's output
+ */
+ public static void toComparableBytes(ByteBuffer value, AbstractType<?> type, byte[] bytes)
+ {
+ if (isInetAddress(type))
+ ByteBufferUtil.copyBytes(value, value.hasArray() ? value.arrayOffset() + value.position() : value.position(), bytes, 0, 16);
+ else if (isBigInteger(type))
+ ByteBufferUtil.copyBytes(value, value.hasArray() ? value.arrayOffset() + value.position() : value.position(), bytes, 0, 20);
+ else if (type instanceof DecimalType)
Review Comment:
I think this can use `isBigDecimal(type)`.
##########
src/java/org/apache/cassandra/index/sai/disk/v1/PerColumnIndexFiles.java:
##########
@@ -50,6 +50,11 @@ public PerColumnIndexFiles(IndexDescriptor indexDescriptor, IndexContext indexCo
files.put(IndexComponent.POSTING_LISTS, indexDescriptor.createPerIndexFileHandle(IndexComponent.POSTING_LISTS, indexContext));
files.put(IndexComponent.TERMS_DATA, indexDescriptor.createPerIndexFileHandle(IndexComponent.TERMS_DATA, indexContext));
}
+ else
+ {
+ files.put(IndexComponent.KD_TREE, indexDescriptor.createPerIndexFileHandle(IndexComponent.KD_TREE, indexContext));
Review Comment:
Nit: The condition of this if block could be `indexContext.isLiteral()` instead of `TypeUtil.isLiteral(indexContext.getValidator())`.
##########
src/java/org/apache/cassandra/index/sai/disk/io/RAMIndexOutput.java:
##########
@@ -70,11 +75,16 @@ public void writeTo(IndexOutput externalOut) throws IOException
externalOut.writeBytes(out.getBytes(), 0, out.getPosition());
}
- public BytesRef getBytes()
+ public BytesRef getBytesRef()
Review Comment:
Nit: I would place getBytes and getBytesRef together, so similar methods stay close to each other.
##########
src/java/org/apache/cassandra/index/sai/disk/v1/segment/SegmentBuilder.java:
##########
@@ -71,10 +73,48 @@
private ByteBuffer minTerm;
private ByteBuffer maxTerm;
+ final AbstractType<?> termComparator;
long totalBytesAllocated;
int rowCount = 0;
int maxSegmentRowId = -1;
+ public static class KDTreeSegmentBuilder extends SegmentBuilder
+ {
+ protected final byte[] buffer;
+ private final BKDTreeRamBuffer kdTreeRamBuffer;
+
+ public KDTreeSegmentBuilder(AbstractType<?> termComparator, NamedMemoryLimiter limiter)
+ {
+ super(termComparator, limiter);
+
+ int typeSize = TypeUtil.fixedSizeOf(termComparator);
+ this.kdTreeRamBuffer = new BKDTreeRamBuffer(typeSize);
+ this.buffer = new byte[typeSize];
+ this.totalBytesAllocated = this.kdTreeRamBuffer.ramBytesUsed();
Review Comment:
Nit: we don't need the `this.`:
```suggestion
kdTreeRamBuffer = new BKDTreeRamBuffer(typeSize);
buffer = new byte[typeSize];
totalBytesAllocated = kdTreeRamBuffer.ramBytesUsed();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org