You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by tk...@apache.org on 2022/09/28 11:10:26 UTC
[ignite-3] branch main updated: IGNITE-17711 Append element count to the end of BinaryTuplePrefix (#1126)
This is an automated email from the ASF dual-hosted git repository.
tkalkirill pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 3e689e86ee IGNITE-17711 Append element count to the end of BinaryTuplePrefix (#1126)
3e689e86ee is described below
commit 3e689e86ee45b797ea0901b5123bbeb37238fcf1
Author: Alexander Polovtcev <al...@gmail.com>
AuthorDate: Wed Sep 28 14:10:21 2022 +0300
IGNITE-17711 Append element count to the end of BinaryTuplePrefix (#1126)
---
.../internal/binarytuple/BinaryTupleBuilder.java | 6 ++-
.../binarytuple/BinaryTuplePrefixBuilder.java | 44 ++++++++++++++++++----
.../ignite/internal/schema/BinaryTuplePrefix.java | 8 ++--
.../internal/schema/BinaryTuplePrefixTest.java | 24 ++++++++++--
.../rocksdb/index/RocksDbSortedIndexStorage.java | 6 +--
5 files changed, 70 insertions(+), 18 deletions(-)
diff --git a/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTupleBuilder.java b/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTupleBuilder.java
index 12c7527f68..965cef3f55 100644
--- a/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTupleBuilder.java
+++ b/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTupleBuilder.java
@@ -574,6 +574,10 @@ public class BinaryTupleBuilder {
* @return Buffer with tuple bytes.
*/
public ByteBuffer build() {
+ return buildInternal().slice().order(ByteOrder.LITTLE_ENDIAN);
+ }
+
+ protected ByteBuffer buildInternal() {
int offset = 0;
int valueSize = buffer.position() - valueBase;
@@ -628,7 +632,7 @@ public class BinaryTupleBuilder {
buffer.put(offset, flags);
- return buffer.flip().position(offset).slice().order(ByteOrder.LITTLE_ENDIAN);
+ return buffer.flip().position(offset);
}
/** Put a byte value to the buffer extending it if needed. */
diff --git a/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTuplePrefixBuilder.java b/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTuplePrefixBuilder.java
index 173765b39f..952fe7d1a4 100644
--- a/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTuplePrefixBuilder.java
+++ b/modules/binary-tuple/src/main/java/org/apache/ignite/internal/binarytuple/BinaryTuplePrefixBuilder.java
@@ -18,6 +18,7 @@
package org.apache.ignite.internal.binarytuple;
import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
/**
* Class for build Binary Tuple Prefixes.
@@ -39,13 +40,26 @@ public class BinaryTuplePrefixBuilder extends BinaryTupleBuilder {
* @param fullNumElements Number of elements in the Binary Tuple Schema.
*/
public BinaryTuplePrefixBuilder(int prefixNumElements, int fullNumElements) {
- super(fullNumElements + 1, true, -1);
+ super(fullNumElements, true);
+
+ this.prefixNumElements = prefixNumElements;
+ }
+
+ /**
+ * Creates a new builder.
+ *
+ * @param prefixNumElements Number of elements in the prefix.
+ * @param fullNumElements Number of elements in the Binary Tuple Schema.
+ * @param totalValueSize Total estimated length of non-NULL values, -1 if not known.
+ */
+ public BinaryTuplePrefixBuilder(int prefixNumElements, int fullNumElements, int totalValueSize) {
+ super(fullNumElements, true, totalValueSize);
this.prefixNumElements = prefixNumElements;
}
@Override
- public ByteBuffer build() {
+ protected ByteBuffer buildInternal() {
int elementIndex = elementIndex();
if (elementIndex != prefixNumElements) {
@@ -58,20 +72,34 @@ public class BinaryTuplePrefixBuilder extends BinaryTupleBuilder {
int numElements = numElements();
// Use nulls instead of the missing elements.
- while (elementIndex() < numElements - 1) {
+ while (elementIndex() < numElements) {
appendNull();
}
- appendInt(prefixNumElements);
-
- ByteBuffer tuple = super.build();
+ ByteBuffer tuple = super.buildInternal();
// Set the flag indicating that this tuple is a prefix.
- byte flags = tuple.get(0);
+ byte flags = tuple.get(tuple.position());
flags |= BinaryTupleCommon.PREFIX_FLAG;
- tuple.put(0, flags);
+ tuple.put(tuple.position(), flags);
+
+ // Append the number of elements to the end of the buffer.
+ // If we have enough space in the original buffer - use it, otherwise allocate a new sufficient buffer.
+ if (tuple.capacity() - tuple.limit() < Integer.BYTES) {
+ tuple = ByteBuffer.allocate(tuple.remaining() + Integer.BYTES)
+ .order(ByteOrder.LITTLE_ENDIAN)
+ .put(tuple)
+ .putInt(prefixNumElements)
+ .flip();
+ } else {
+ int prevLimit = tuple.limit();
+
+ tuple
+ .limit(prevLimit + Integer.BYTES)
+ .putInt(prevLimit, prefixNumElements);
+ }
return tuple;
}
diff --git a/modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTuplePrefix.java b/modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTuplePrefix.java
index 32e23dda2f..d7ef4817f5 100644
--- a/modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTuplePrefix.java
+++ b/modules/schema/src/main/java/org/apache/ignite/internal/schema/BinaryTuplePrefix.java
@@ -39,7 +39,7 @@ public class BinaryTuplePrefix extends BinaryTupleReader implements InternalTupl
* @param bytes Serialized representation of a Binary Tuple Prefix.
*/
public BinaryTuplePrefix(BinaryTupleSchema schema, byte[] bytes) {
- super(schema.elementCount() + 1, bytes);
+ super(schema.elementCount(), bytes);
this.schema = schema;
}
@@ -50,7 +50,7 @@ public class BinaryTuplePrefix extends BinaryTupleReader implements InternalTupl
* @param buffer Serialized representation of a Binary Tuple Prefix.
*/
public BinaryTuplePrefix(BinaryTupleSchema schema, ByteBuffer buffer) {
- super(schema.elementCount() + 1, buffer);
+ super(schema.elementCount(), buffer);
this.schema = schema;
}
@@ -66,6 +66,8 @@ public class BinaryTuplePrefix extends BinaryTupleReader implements InternalTupl
@Override
public int elementCount() {
- return intValue(super.elementCount() - 1);
+ ByteBuffer buffer = byteBuffer();
+
+ return buffer.getInt(buffer.limit() - Integer.BYTES);
}
}
diff --git a/modules/schema/src/test/java/org/apache/ignite/internal/schema/BinaryTuplePrefixTest.java b/modules/schema/src/test/java/org/apache/ignite/internal/schema/BinaryTuplePrefixTest.java
index 72574b7ced..993feb92e1 100644
--- a/modules/schema/src/test/java/org/apache/ignite/internal/schema/BinaryTuplePrefixTest.java
+++ b/modules/schema/src/test/java/org/apache/ignite/internal/schema/BinaryTuplePrefixTest.java
@@ -48,11 +48,10 @@ public class BinaryTuplePrefixTest {
new Element(NativeTypes.DOUBLE, false)
});
- var builder = new BinaryTuplePrefixBuilder(3, 5);
-
LocalDate date = LocalDate.now();
- ByteBuffer tuple = builder.appendInt(42)
+ ByteBuffer tuple = new BinaryTuplePrefixBuilder(3, schema.elementCount())
+ .appendInt(42)
.appendString("foobar")
.appendDate(date)
.build();
@@ -71,6 +70,25 @@ public class BinaryTuplePrefixTest {
assertThat(prefix.doubleValueBoxed(4), is(nullValue()));
}
+ /**
+ * Tests a corner case when a new internal buffer needs to be allocated to add the count value.
+ */
+ @Test
+ public void testInternalBufferReallocation() {
+ BinaryTupleSchema schema = BinaryTupleSchema.create(new Element[]{
+ new Element(NativeTypes.INT32, false)
+ });
+
+ ByteBuffer tuple = new BinaryTuplePrefixBuilder(1, schema.elementCount(), 4)
+ .appendInt(Integer.MAX_VALUE)
+ .build();
+
+ var prefix = new BinaryTuplePrefix(schema, tuple);
+
+ assertThat(prefix.count(), is(1));
+ assertThat(prefix.intValue(0), is(Integer.MAX_VALUE));
+ }
+
/**
* Tests construction of an invalid prefix.
*/
diff --git a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
index 36321b8b6c..927be72c10 100644
--- a/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
+++ b/modules/storage-rocksdb/src/main/java/org/apache/ignite/internal/storage/rocksdb/index/RocksDbSortedIndexStorage.java
@@ -213,6 +213,8 @@ public class RocksDbSortedIndexStorage implements SortedIndexStorage {
private Predicate<ByteBuffer> startsWith(BinaryTuplePrefix prefix) {
var comparator = new BinaryTupleComparator(descriptor);
+ ByteBuffer prefixBuffer = prefix.byteBuffer();
+
return key -> {
// First, compare the partitionIDs.
boolean partitionIdCompare = key.getShort(0) == partitionStorage.partitionId();
@@ -222,9 +224,7 @@ public class RocksDbSortedIndexStorage implements SortedIndexStorage {
}
// Finally, compare the remaining parts of the tuples.
- // TODO: This part may be optimized by comparing binary tuple representations directly. However, currently BinaryTuple prefixes
- // are not binary compatible with regular tuples. See https://issues.apache.org/jira/browse/IGNITE-17711.
- return comparator.compare(prefix.byteBuffer(), binaryTupleSlice(key)) == 0;
+ return comparator.compare(prefixBuffer, binaryTupleSlice(key)) == 0;
};
}