Fix AbstractTokenTreeBuilder#serializedSize() when there is a single leaf and overflow collisions
patch by Jordan West; reviewed by jasobrown for CASSANDRA-13869 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/436761ed Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/436761ed Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/436761ed Branch: refs/heads/trunk Commit: 436761ed6afbfdc333c6642f6b4418d44712a0b7 Parents: 2a93c22 Author: Jordan West <jorda...@gmail.com> Authored: Wed Sep 13 23:37:02 2017 -0700 Committer: Jason Brown <jasedbr...@gmail.com> Committed: Fri Sep 15 14:05:02 2017 -0700 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../sasi/disk/AbstractTokenTreeBuilder.java | 8 +++++- .../index/sasi/disk/TokenTreeBuilder.java | 6 ++-- .../index/sasi/disk/TokenTreeTest.java | 30 +++++++++++++------- 4 files changed, 31 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/436761ed/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 33055e5..f0c05bf 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.11.1 + * AbstractTokenTreeBuilder#serializedSize returns wrong value when there is a single leaf and overflow collisions (CASSANDRA-13869) * Add a compaction option to TWCS to ignore sstables overlapping checks (CASSANDRA-13418) * BTree.Builder memory leak (CASSANDRA-13754) * Revert CASSANDRA-10368 of supporting non-pk column filtering due to correctness (CASSANDRA-13798) http://git-wip-us.apache.org/repos/asf/cassandra/blob/436761ed/src/java/org/apache/cassandra/index/sasi/disk/AbstractTokenTreeBuilder.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/index/sasi/disk/AbstractTokenTreeBuilder.java b/src/java/org/apache/cassandra/index/sasi/disk/AbstractTokenTreeBuilder.java index 18994de..ae1024f 100644 --- a/src/java/org/apache/cassandra/index/sasi/disk/AbstractTokenTreeBuilder.java +++ b/src/java/org/apache/cassandra/index/sasi/disk/AbstractTokenTreeBuilder.java @@ -65,7 +65,9 @@ public abstract class AbstractTokenTreeBuilder implements TokenTreeBuilder public int serializedSize() { if (numBlocks == 1) - return (BLOCK_HEADER_BYTES + ((int) tokenCount * 16)); + return BLOCK_HEADER_BYTES + + ((int) tokenCount * BLOCK_ENTRY_BYTES) + + (((Leaf) root).overflowCollisionCount() * OVERFLOW_ENTRY_BYTES); else return numBlocks * BLOCK_BYTES; } @@ -263,6 +265,10 @@ public abstract class AbstractTokenTreeBuilder implements TokenTreeBuilder return 0; } + public int overflowCollisionCount() { + return overflowCollisions == null ? 0 : overflowCollisions.size(); + } + protected void serializeOverflowCollisions(ByteBuffer buf) { if (overflowCollisions != null) http://git-wip-us.apache.org/repos/asf/cassandra/blob/436761ed/src/java/org/apache/cassandra/index/sasi/disk/TokenTreeBuilder.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/index/sasi/disk/TokenTreeBuilder.java b/src/java/org/apache/cassandra/index/sasi/disk/TokenTreeBuilder.java index 2210964..29cecc8 100644 --- a/src/java/org/apache/cassandra/index/sasi/disk/TokenTreeBuilder.java +++ b/src/java/org/apache/cassandra/index/sasi/disk/TokenTreeBuilder.java @@ -29,9 +29,11 @@ public interface TokenTreeBuilder extends Iterable<Pair<Long, LongSet>> { int BLOCK_BYTES = 4096; int BLOCK_HEADER_BYTES = 64; + int BLOCK_ENTRY_BYTES = 2 * Long.BYTES; int OVERFLOW_TRAILER_BYTES = 64; - int OVERFLOW_TRAILER_CAPACITY = OVERFLOW_TRAILER_BYTES / 8; - int TOKENS_PER_BLOCK = (BLOCK_BYTES - BLOCK_HEADER_BYTES - OVERFLOW_TRAILER_BYTES) / 16; + int OVERFLOW_ENTRY_BYTES = Long.BYTES; + int OVERFLOW_TRAILER_CAPACITY = OVERFLOW_TRAILER_BYTES / OVERFLOW_ENTRY_BYTES; + int TOKENS_PER_BLOCK = (BLOCK_BYTES - BLOCK_HEADER_BYTES - OVERFLOW_TRAILER_BYTES) / BLOCK_ENTRY_BYTES; long MAX_OFFSET = (1L << 47) - 1; // 48 bits for (signed) offset byte LAST_LEAF_SHIFT = 1; byte SHARED_HEADER_BYTES = 19; http://git-wip-us.apache.org/repos/asf/cassandra/blob/436761ed/test/unit/org/apache/cassandra/index/sasi/disk/TokenTreeTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/index/sasi/disk/TokenTreeTest.java b/test/unit/org/apache/cassandra/index/sasi/disk/TokenTreeTest.java index 927e165..3d0850a 100644 --- a/test/unit/org/apache/cassandra/index/sasi/disk/TokenTreeTest.java +++ b/test/unit/org/apache/cassandra/index/sasi/disk/TokenTreeTest.java @@ -87,25 +87,29 @@ public class TokenTreeTest put(i, singleOffset); }}; - final static SortedMap<Long, LongSet> collidingTokensMap = new TreeMap<Long, LongSet>() - {{ - put(1L, singleOffset); put(7L, singleOffset); put(8L, singleOffset); - }}; + @FunctionalInterface + private static interface CheckedConsumer<C> { + public void accept(C c) throws Exception; + } - final static SortedMap<Long, LongSet> tokens = bigTokensMap; + final static List<SortedMap<Long, LongSet>> tokenMaps = Arrays.asList(simpleTokenMap, bigTokensMap); + private void forAllTokenMaps(CheckedConsumer<SortedMap<Long, LongSet>> c) throws Exception { + for (SortedMap<Long, LongSet> tokens : tokenMaps) + c.accept(tokens); + } final static SequentialWriterOption DEFAULT_OPT = SequentialWriterOption.newBuilder().bufferSize(4096).build(); @Test public void testSerializedSizeDynamic() throws Exception { - testSerializedSize(new DynamicTokenTreeBuilder(tokens)); + forAllTokenMaps(tokens -> testSerializedSize(new DynamicTokenTreeBuilder(tokens))); } @Test public void testSerializedSizeStatic() throws Exception { - testSerializedSize(new StaticTokenTreeBuilder(new FakeCombinedTerm(tokens))); + forAllTokenMaps(tokens -> testSerializedSize(new StaticTokenTreeBuilder(new FakeCombinedTerm(tokens)))); } @@ -129,13 +133,14 @@ public class TokenTreeTest @Test public void buildSerializeAndIterateDynamic() throws Exception { - buildSerializeAndIterate(new DynamicTokenTreeBuilder(simpleTokenMap), simpleTokenMap); + forAllTokenMaps(tokens -> buildSerializeAndIterate(new DynamicTokenTreeBuilder(tokens), tokens)); } @Test public void buildSerializeAndIterateStatic() throws Exception { - buildSerializeAndIterate(new StaticTokenTreeBuilder(new FakeCombinedTerm(tokens)), tokens); + forAllTokenMaps(tokens -> + buildSerializeAndIterate(new StaticTokenTreeBuilder(new FakeCombinedTerm(tokens)), tokens)); } @@ -207,15 +212,18 @@ public class TokenTreeTest @Test public void buildSerializeIterateAndSkipDynamic() throws Exception { - buildSerializeIterateAndSkip(new DynamicTokenTreeBuilder(tokens), tokens); + forAllTokenMaps(tokens -> buildSerializeIterateAndSkip(new DynamicTokenTreeBuilder(tokens), tokens)); } @Test public void buildSerializeIterateAndSkipStatic() throws Exception { - buildSerializeIterateAndSkip(new StaticTokenTreeBuilder(new FakeCombinedTerm(tokens)), tokens); + forAllTokenMaps(tokens -> + buildSerializeIterateAndSkip(new StaticTokenTreeBuilder(new FakeCombinedTerm(tokens)), tokens)); } + // works with maps other than bigTokensMap but skips to a rather large token + // so likely for maps other than bigTokensMap skipping is not tested by this. public void buildSerializeIterateAndSkip(TokenTreeBuilder builder, SortedMap<Long, LongSet> tokens) throws Exception { builder.finish(); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
