You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2015/12/23 15:19:05 UTC
[1/3] cassandra git commit: Fix assertion error when reading static
on an indexed sstable
Repository: cassandra
Updated Branches:
refs/heads/cassandra-3.0 796db6e09 -> 8bc567b3c
refs/heads/trunk e5b6a9c8b -> 04a99ab84
Fix assertion error when reading static on an indexed sstable
patch by slebresne; reviewed by blambov for CASSANDRA-10903
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/8bc567b3
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/8bc567b3
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/8bc567b3
Branch: refs/heads/cassandra-3.0
Commit: 8bc567b3ca40199f6d4a7b6f32971f2fea56a6b9
Parents: 796db6e
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Mon Dec 21 12:14:46 2015 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Dec 23 15:15:14 2015 +0100
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../columniterator/AbstractSSTableIterator.java | 37 ++++-----
.../db/columniterator/SSTableIterator.java | 38 +++------
.../columniterator/SSTableReversedIterator.java | 28 ++-----
.../cql3/QueryWithIndexedSSTableTest.java | 84 ++++++++++++++++++++
5 files changed, 121 insertions(+), 67 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8bc567b3/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 7794c96..a669b17 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.0.3
+ * Fix potential assertion error when reading static columns (CASSANDRA-0903)
* Avoid NoSuchElementException when executing empty batch (CASSANDRA-10711)
* Avoid building PartitionUpdate in toString (CASSANDRA-10897)
* Reduce heap spent when receiving many SSTables (CASSANDRA-10797)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8bc567b3/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java b/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java
index 5f280d7..8ac3dcb 100644
--- a/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java
+++ b/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java
@@ -99,14 +99,14 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
// Note that this needs to be called after file != null and after the partitionDeletion has been set, but before readStaticRow
// (since it uses it) so we can't move that up (but we'll be able to simplify as soon as we drop support for the old file format).
- this.reader = needsReader ? createReader(indexEntry, file, true, shouldCloseFile) : null;
+ this.reader = needsReader ? createReader(indexEntry, file, shouldCloseFile) : null;
this.staticRow = readStaticRow(sstable, file, helper, columns.fetchedColumns().statics, isForThrift, reader == null ? null : reader.deserializer);
}
else
{
this.partitionLevelDeletion = indexEntry.deletionTime();
this.staticRow = Rows.EMPTY_STATIC_ROW;
- this.reader = needsReader ? createReader(indexEntry, file, false, shouldCloseFile) : null;
+ this.reader = needsReader ? createReader(indexEntry, file, shouldCloseFile) : null;
}
if (reader == null && file != null && shouldCloseFile)
@@ -180,7 +180,7 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
}
}
- protected abstract Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile);
+ protected abstract Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean shouldCloseFile);
public CFMetaData metadata()
{
@@ -291,19 +291,13 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
// Records the currently open range tombstone (if any)
protected DeletionTime openMarker = null;
- // !isInit means we have never seeked in the file and thus should seek before reading anything
- protected boolean isInit;
-
- protected Reader(FileDataInput file, boolean isInit, boolean shouldCloseFile)
+ protected Reader(FileDataInput file, boolean shouldCloseFile)
{
this.file = file;
- this.isInit = isInit;
this.shouldCloseFile = shouldCloseFile;
if (file != null)
createDeserializer();
- else
- assert !isInit;
}
private void createDeserializer()
@@ -343,12 +337,6 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
{
try
{
- if (!isInit)
- {
- init();
- isInit = true;
- }
-
return hasNextInternal();
}
catch (IOException e)
@@ -387,9 +375,6 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
}
}
- // Called is hasNext() is called but we haven't been yet initialized
- protected abstract void init() throws IOException;
-
// Set the reader so its hasNext/next methods return values within the provided slice
public abstract void setForSlice(Slice slice) throws IOException;
@@ -458,9 +443,21 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
}
// Update the block idx based on the current reader position if we're past the current block.
+ // This only makes sense for forward iteration (for reverse ones, when we reach the end of a block we
+ // should seek to the previous one, not update the index state and continue).
public void updateBlock() throws IOException
{
- assert currentIndexIdx >= 0;
+ assert !reversed;
+
+ // If we get here with currentBlockIdx < 0, it means setToBlock() has never been called, so it means
+ // we're about to read from the beginning of the partition, but haven't "prepared" the IndexState yet.
+ // Do so by setting us on the first block.
+ if (currentIndexIdx < 0)
+ {
+ setToBlock(0);
+ return;
+ }
+
while (currentIndexIdx + 1 < indexes.size() && isPastCurrentBlock())
{
reader.openMarker = currentIndex().endOpenMarker;
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8bc567b3/src/java/org/apache/cassandra/db/columniterator/SSTableIterator.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/columniterator/SSTableIterator.java b/src/java/org/apache/cassandra/db/columniterator/SSTableIterator.java
index 3536d65..0409310 100644
--- a/src/java/org/apache/cassandra/db/columniterator/SSTableIterator.java
+++ b/src/java/org/apache/cassandra/db/columniterator/SSTableIterator.java
@@ -46,11 +46,11 @@ public class SSTableIterator extends AbstractSSTableIterator
super(sstable, file, key, indexEntry, columns, isForThrift);
}
- protected Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ protected Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean shouldCloseFile)
{
return indexEntry.isIndexed()
- ? new ForwardIndexedReader(indexEntry, file, isAtPartitionStart, shouldCloseFile)
- : new ForwardReader(file, isAtPartitionStart, shouldCloseFile);
+ ? new ForwardIndexedReader(indexEntry, file, shouldCloseFile)
+ : new ForwardReader(file, shouldCloseFile);
}
public boolean isReverseOrder()
@@ -70,16 +70,9 @@ public class SSTableIterator extends AbstractSSTableIterator
protected boolean sliceDone; // set to true once we know we have no more result for the slice. This is in particular
// used by the indexed reader when we know we can't have results based on the index.
- private ForwardReader(FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ private ForwardReader(FileDataInput file, boolean shouldCloseFile)
{
- super(file, isAtPartitionStart, shouldCloseFile);
- }
-
- protected void init() throws IOException
- {
- // We should always have been initialized (at the beginning of the partition). Only indexed readers may
- // have to initialize.
- throw new IllegalStateException();
+ super(file, shouldCloseFile);
}
public void setForSlice(Slice slice) throws IOException
@@ -95,6 +88,8 @@ public class SSTableIterator extends AbstractSSTableIterator
// Return what should be returned at the end of this, or null if nothing should.
private Unfiltered handlePreSliceData() throws IOException
{
+ assert deserializer != null;
+
// Note that the following comparison is not strict. The reason is that the only cases
// where it can be == is if the "next" is a RT start marker (either a '[' of a ')[' boundary),
// and if we had a strict inequality and an open RT marker before this, we would issue
@@ -126,6 +121,8 @@ public class SSTableIterator extends AbstractSSTableIterator
// if we're done with the slice.
protected Unfiltered computeNext() throws IOException
{
+ assert deserializer != null;
+
if (!deserializer.hasNext() || deserializer.compareNextTo(end) > 0)
return null;
@@ -143,8 +140,6 @@ public class SSTableIterator extends AbstractSSTableIterator
if (sliceDone)
return false;
- assert deserializer != null;
-
if (start != null)
{
Unfiltered unfiltered = handlePreSliceData();
@@ -187,28 +182,18 @@ public class SSTableIterator extends AbstractSSTableIterator
private int lastBlockIdx; // the last index block that has data for the current query
- private ForwardIndexedReader(RowIndexEntry indexEntry, FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ private ForwardIndexedReader(RowIndexEntry indexEntry, FileDataInput file, boolean shouldCloseFile)
{
- super(file, isAtPartitionStart, shouldCloseFile);
+ super(file, shouldCloseFile);
this.indexState = new IndexState(this, sstable.metadata.comparator, indexEntry, false);
this.lastBlockIdx = indexState.blocksCount(); // if we never call setForSlice, that's where we want to stop
}
@Override
- protected void init() throws IOException
- {
- // If this is called, it means we're calling hasNext() before any call to setForSlice. Which means
- // we're reading everything from the beginning. So just set us up at the beginning of the first block.
- indexState.setToBlock(0);
- }
-
- @Override
public void setForSlice(Slice slice) throws IOException
{
super.setForSlice(slice);
- isInit = true;
-
// if our previous slicing already got us the biggest row in the sstable, we're done
if (indexState.isDone())
{
@@ -265,6 +250,7 @@ public class SSTableIterator extends AbstractSSTableIterator
protected Unfiltered computeNext() throws IOException
{
// Our previous read might have made us cross an index block boundary. If so, update our informations.
+ // If we read from the beginning of the partition, this is also what will initialize the index state.
indexState.updateBlock();
// Return the next unfiltered unless we've reached the end, or we're beyond our slice
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8bc567b3/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java b/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java
index 66c32ee..14cec36 100644
--- a/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java
+++ b/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java
@@ -49,11 +49,11 @@ public class SSTableReversedIterator extends AbstractSSTableIterator
super(sstable, file, key, indexEntry, columns, isForThrift);
}
- protected Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ protected Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean shouldCloseFile)
{
return indexEntry.isIndexed()
- ? new ReverseIndexedReader(indexEntry, file, isAtPartitionStart, shouldCloseFile)
- : new ReverseReader(file, isAtPartitionStart, shouldCloseFile);
+ ? new ReverseIndexedReader(indexEntry, file, shouldCloseFile)
+ : new ReverseReader(file, shouldCloseFile);
}
public boolean isReverseOrder()
@@ -66,9 +66,9 @@ public class SSTableReversedIterator extends AbstractSSTableIterator
protected ReusablePartitionData buffer;
protected Iterator<Unfiltered> iterator;
- private ReverseReader(FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ private ReverseReader(FileDataInput file, boolean shouldCloseFile)
{
- super(file, isAtPartitionStart, shouldCloseFile);
+ super(file, shouldCloseFile);
}
protected ReusablePartitionData createBuffer(int blocksCount)
@@ -100,13 +100,6 @@ public class SSTableReversedIterator extends AbstractSSTableIterator
return new ReusablePartitionData(metadata(), partitionKey(), columns(), estimatedRowCount);
}
- protected void init() throws IOException
- {
- // We should always have been initialized (at the beginning of the partition). Only indexed readers may
- // have to initialize.
- throw new IllegalStateException();
- }
-
public void setForSlice(Slice slice) throws IOException
{
// If we have read the data, just create the iterator for the slice. Otherwise, read the data.
@@ -212,23 +205,16 @@ public class SSTableReversedIterator extends AbstractSSTableIterator
// The last index block to consider for the slice
private int lastBlockIdx;
- private ReverseIndexedReader(RowIndexEntry indexEntry, FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ private ReverseIndexedReader(RowIndexEntry indexEntry, FileDataInput file, boolean shouldCloseFile)
{
- super(file, isAtPartitionStart, shouldCloseFile);
+ super(file, shouldCloseFile);
this.indexState = new IndexState(this, sstable.metadata.comparator, indexEntry, true);
}
- protected void init() throws IOException
- {
- // This is actually a no-op, because if we call hasNext without having called setForSlice, then ReverseReader.hasNextInternal
- // will call setForSlice(Slice.ALL) which does the right thing.
- }
-
@Override
public void setForSlice(Slice slice) throws IOException
{
this.slice = slice;
- isInit = true;
// if our previous slicing already got us past the beginning of the sstable, we're done
if (indexState.isDone())
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8bc567b3/test/unit/org/apache/cassandra/cql3/QueryWithIndexedSSTableTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/QueryWithIndexedSSTableTest.java b/test/unit/org/apache/cassandra/cql3/QueryWithIndexedSSTableTest.java
new file mode 100644
index 0000000..4838392
--- /dev/null
+++ b/test/unit/org/apache/cassandra/cql3/QueryWithIndexedSSTableTest.java
@@ -0,0 +1,84 @@
+/*
+ * 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.cql3;
+
+import java.util.Random;
+
+import org.junit.Test;
+
+import org.apache.cassandra.Util;
+import org.apache.cassandra.db.DecoratedKey;
+import org.apache.cassandra.db.RowIndexEntry;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
+import org.apache.cassandra.utils.ByteBufferUtil;
+
+public class QueryWithIndexedSSTableTest extends CQLTester
+{
+ @Test
+ public void queryIndexedSSTableTest() throws Throwable
+ {
+ // That test reproduces the bug from CASSANDRA-10903 and the fact we have a static column is
+ // relevant to that reproduction in particular as it forces a slightly different code path that
+ // if there wasn't a static.
+
+ int ROWS = 1000;
+ int VALUE_LENGTH = 100;
+
+ createTable("CREATE TABLE %s (k int, t int, s text static, v text, PRIMARY KEY (k, t))");
+
+ // We create a partition that is big enough that the underlying sstable will be indexed
+ // For that, we use a large-ish number of row, and a value that isn't too small.
+ String text = makeRandomTest(VALUE_LENGTH);
+ for (int i = 0; i < ROWS; i++)
+ execute("INSERT INTO %s(k, t, v) VALUES (?, ?, ?)", 0, i, text + i);
+
+ flush();
+ compact();
+
+ // Sanity check that we're testing what we want to test, that is that we're reading from an indexed
+ // sstable. Note that we'll almost surely have a single indexed sstable in practice, but it's theorically
+ // possible for a compact strategy to yield more than that and as long as one is indexed we're pretty
+ // much testing what we want. If this check ever fails on some specific setting, we'll have to either
+ // tweak ROWS and VALUE_LENGTH, or skip the test on those settings.
+ DecoratedKey dk = Util.dk(ByteBufferUtil.bytes(0));
+ boolean hasIndexed = false;
+ for (SSTableReader sstable : getCurrentColumnFamilyStore().getLiveSSTables())
+ {
+ RowIndexEntry indexEntry = sstable.getPosition(dk, SSTableReader.Operator.EQ);
+ hasIndexed |= indexEntry != null && indexEntry.isIndexed();
+ }
+ assert hasIndexed;
+
+ assertRowCount(execute("SELECT s FROM %s WHERE k = ?", 0), ROWS);
+ assertRowCount(execute("SELECT s FROM %s WHERE k = ? ORDER BY t DESC", 0), ROWS);
+
+ assertRowCount(execute("SELECT DISTINCT s FROM %s WHERE k = ?", 0), 1);
+ assertRowCount(execute("SELECT DISTINCT s FROM %s WHERE k = ? ORDER BY t DESC", 0), 1);
+ }
+
+ // Creates a random string
+ public static String makeRandomTest(int length)
+ {
+ Random random = new Random();
+ char[] chars = new char[26];
+ int i = 0;
+ for (char c = 'a'; c <= 'z'; c++)
+ chars[i++] = c;
+ return new String(chars);
+ }
+}
[2/3] cassandra git commit: Fix assertion error when reading static
on an indexed sstable
Posted by sl...@apache.org.
Fix assertion error when reading static on an indexed sstable
patch by slebresne; reviewed by blambov for CASSANDRA-10903
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/8bc567b3
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/8bc567b3
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/8bc567b3
Branch: refs/heads/trunk
Commit: 8bc567b3ca40199f6d4a7b6f32971f2fea56a6b9
Parents: 796db6e
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Mon Dec 21 12:14:46 2015 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Dec 23 15:15:14 2015 +0100
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../columniterator/AbstractSSTableIterator.java | 37 ++++-----
.../db/columniterator/SSTableIterator.java | 38 +++------
.../columniterator/SSTableReversedIterator.java | 28 ++-----
.../cql3/QueryWithIndexedSSTableTest.java | 84 ++++++++++++++++++++
5 files changed, 121 insertions(+), 67 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8bc567b3/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 7794c96..a669b17 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.0.3
+ * Fix potential assertion error when reading static columns (CASSANDRA-0903)
* Avoid NoSuchElementException when executing empty batch (CASSANDRA-10711)
* Avoid building PartitionUpdate in toString (CASSANDRA-10897)
* Reduce heap spent when receiving many SSTables (CASSANDRA-10797)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8bc567b3/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java b/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java
index 5f280d7..8ac3dcb 100644
--- a/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java
+++ b/src/java/org/apache/cassandra/db/columniterator/AbstractSSTableIterator.java
@@ -99,14 +99,14 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
// Note that this needs to be called after file != null and after the partitionDeletion has been set, but before readStaticRow
// (since it uses it) so we can't move that up (but we'll be able to simplify as soon as we drop support for the old file format).
- this.reader = needsReader ? createReader(indexEntry, file, true, shouldCloseFile) : null;
+ this.reader = needsReader ? createReader(indexEntry, file, shouldCloseFile) : null;
this.staticRow = readStaticRow(sstable, file, helper, columns.fetchedColumns().statics, isForThrift, reader == null ? null : reader.deserializer);
}
else
{
this.partitionLevelDeletion = indexEntry.deletionTime();
this.staticRow = Rows.EMPTY_STATIC_ROW;
- this.reader = needsReader ? createReader(indexEntry, file, false, shouldCloseFile) : null;
+ this.reader = needsReader ? createReader(indexEntry, file, shouldCloseFile) : null;
}
if (reader == null && file != null && shouldCloseFile)
@@ -180,7 +180,7 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
}
}
- protected abstract Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile);
+ protected abstract Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean shouldCloseFile);
public CFMetaData metadata()
{
@@ -291,19 +291,13 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
// Records the currently open range tombstone (if any)
protected DeletionTime openMarker = null;
- // !isInit means we have never seeked in the file and thus should seek before reading anything
- protected boolean isInit;
-
- protected Reader(FileDataInput file, boolean isInit, boolean shouldCloseFile)
+ protected Reader(FileDataInput file, boolean shouldCloseFile)
{
this.file = file;
- this.isInit = isInit;
this.shouldCloseFile = shouldCloseFile;
if (file != null)
createDeserializer();
- else
- assert !isInit;
}
private void createDeserializer()
@@ -343,12 +337,6 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
{
try
{
- if (!isInit)
- {
- init();
- isInit = true;
- }
-
return hasNextInternal();
}
catch (IOException e)
@@ -387,9 +375,6 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
}
}
- // Called is hasNext() is called but we haven't been yet initialized
- protected abstract void init() throws IOException;
-
// Set the reader so its hasNext/next methods return values within the provided slice
public abstract void setForSlice(Slice slice) throws IOException;
@@ -458,9 +443,21 @@ abstract class AbstractSSTableIterator implements SliceableUnfilteredRowIterator
}
// Update the block idx based on the current reader position if we're past the current block.
+ // This only makes sense for forward iteration (for reverse ones, when we reach the end of a block we
+ // should seek to the previous one, not update the index state and continue).
public void updateBlock() throws IOException
{
- assert currentIndexIdx >= 0;
+ assert !reversed;
+
+ // If we get here with currentBlockIdx < 0, it means setToBlock() has never been called, so it means
+ // we're about to read from the beginning of the partition, but haven't "prepared" the IndexState yet.
+ // Do so by setting us on the first block.
+ if (currentIndexIdx < 0)
+ {
+ setToBlock(0);
+ return;
+ }
+
while (currentIndexIdx + 1 < indexes.size() && isPastCurrentBlock())
{
reader.openMarker = currentIndex().endOpenMarker;
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8bc567b3/src/java/org/apache/cassandra/db/columniterator/SSTableIterator.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/columniterator/SSTableIterator.java b/src/java/org/apache/cassandra/db/columniterator/SSTableIterator.java
index 3536d65..0409310 100644
--- a/src/java/org/apache/cassandra/db/columniterator/SSTableIterator.java
+++ b/src/java/org/apache/cassandra/db/columniterator/SSTableIterator.java
@@ -46,11 +46,11 @@ public class SSTableIterator extends AbstractSSTableIterator
super(sstable, file, key, indexEntry, columns, isForThrift);
}
- protected Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ protected Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean shouldCloseFile)
{
return indexEntry.isIndexed()
- ? new ForwardIndexedReader(indexEntry, file, isAtPartitionStart, shouldCloseFile)
- : new ForwardReader(file, isAtPartitionStart, shouldCloseFile);
+ ? new ForwardIndexedReader(indexEntry, file, shouldCloseFile)
+ : new ForwardReader(file, shouldCloseFile);
}
public boolean isReverseOrder()
@@ -70,16 +70,9 @@ public class SSTableIterator extends AbstractSSTableIterator
protected boolean sliceDone; // set to true once we know we have no more result for the slice. This is in particular
// used by the indexed reader when we know we can't have results based on the index.
- private ForwardReader(FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ private ForwardReader(FileDataInput file, boolean shouldCloseFile)
{
- super(file, isAtPartitionStart, shouldCloseFile);
- }
-
- protected void init() throws IOException
- {
- // We should always have been initialized (at the beginning of the partition). Only indexed readers may
- // have to initialize.
- throw new IllegalStateException();
+ super(file, shouldCloseFile);
}
public void setForSlice(Slice slice) throws IOException
@@ -95,6 +88,8 @@ public class SSTableIterator extends AbstractSSTableIterator
// Return what should be returned at the end of this, or null if nothing should.
private Unfiltered handlePreSliceData() throws IOException
{
+ assert deserializer != null;
+
// Note that the following comparison is not strict. The reason is that the only cases
// where it can be == is if the "next" is a RT start marker (either a '[' of a ')[' boundary),
// and if we had a strict inequality and an open RT marker before this, we would issue
@@ -126,6 +121,8 @@ public class SSTableIterator extends AbstractSSTableIterator
// if we're done with the slice.
protected Unfiltered computeNext() throws IOException
{
+ assert deserializer != null;
+
if (!deserializer.hasNext() || deserializer.compareNextTo(end) > 0)
return null;
@@ -143,8 +140,6 @@ public class SSTableIterator extends AbstractSSTableIterator
if (sliceDone)
return false;
- assert deserializer != null;
-
if (start != null)
{
Unfiltered unfiltered = handlePreSliceData();
@@ -187,28 +182,18 @@ public class SSTableIterator extends AbstractSSTableIterator
private int lastBlockIdx; // the last index block that has data for the current query
- private ForwardIndexedReader(RowIndexEntry indexEntry, FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ private ForwardIndexedReader(RowIndexEntry indexEntry, FileDataInput file, boolean shouldCloseFile)
{
- super(file, isAtPartitionStart, shouldCloseFile);
+ super(file, shouldCloseFile);
this.indexState = new IndexState(this, sstable.metadata.comparator, indexEntry, false);
this.lastBlockIdx = indexState.blocksCount(); // if we never call setForSlice, that's where we want to stop
}
@Override
- protected void init() throws IOException
- {
- // If this is called, it means we're calling hasNext() before any call to setForSlice. Which means
- // we're reading everything from the beginning. So just set us up at the beginning of the first block.
- indexState.setToBlock(0);
- }
-
- @Override
public void setForSlice(Slice slice) throws IOException
{
super.setForSlice(slice);
- isInit = true;
-
// if our previous slicing already got us the biggest row in the sstable, we're done
if (indexState.isDone())
{
@@ -265,6 +250,7 @@ public class SSTableIterator extends AbstractSSTableIterator
protected Unfiltered computeNext() throws IOException
{
// Our previous read might have made us cross an index block boundary. If so, update our informations.
+ // If we read from the beginning of the partition, this is also what will initialize the index state.
indexState.updateBlock();
// Return the next unfiltered unless we've reached the end, or we're beyond our slice
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8bc567b3/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java b/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java
index 66c32ee..14cec36 100644
--- a/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java
+++ b/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java
@@ -49,11 +49,11 @@ public class SSTableReversedIterator extends AbstractSSTableIterator
super(sstable, file, key, indexEntry, columns, isForThrift);
}
- protected Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ protected Reader createReader(RowIndexEntry indexEntry, FileDataInput file, boolean shouldCloseFile)
{
return indexEntry.isIndexed()
- ? new ReverseIndexedReader(indexEntry, file, isAtPartitionStart, shouldCloseFile)
- : new ReverseReader(file, isAtPartitionStart, shouldCloseFile);
+ ? new ReverseIndexedReader(indexEntry, file, shouldCloseFile)
+ : new ReverseReader(file, shouldCloseFile);
}
public boolean isReverseOrder()
@@ -66,9 +66,9 @@ public class SSTableReversedIterator extends AbstractSSTableIterator
protected ReusablePartitionData buffer;
protected Iterator<Unfiltered> iterator;
- private ReverseReader(FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ private ReverseReader(FileDataInput file, boolean shouldCloseFile)
{
- super(file, isAtPartitionStart, shouldCloseFile);
+ super(file, shouldCloseFile);
}
protected ReusablePartitionData createBuffer(int blocksCount)
@@ -100,13 +100,6 @@ public class SSTableReversedIterator extends AbstractSSTableIterator
return new ReusablePartitionData(metadata(), partitionKey(), columns(), estimatedRowCount);
}
- protected void init() throws IOException
- {
- // We should always have been initialized (at the beginning of the partition). Only indexed readers may
- // have to initialize.
- throw new IllegalStateException();
- }
-
public void setForSlice(Slice slice) throws IOException
{
// If we have read the data, just create the iterator for the slice. Otherwise, read the data.
@@ -212,23 +205,16 @@ public class SSTableReversedIterator extends AbstractSSTableIterator
// The last index block to consider for the slice
private int lastBlockIdx;
- private ReverseIndexedReader(RowIndexEntry indexEntry, FileDataInput file, boolean isAtPartitionStart, boolean shouldCloseFile)
+ private ReverseIndexedReader(RowIndexEntry indexEntry, FileDataInput file, boolean shouldCloseFile)
{
- super(file, isAtPartitionStart, shouldCloseFile);
+ super(file, shouldCloseFile);
this.indexState = new IndexState(this, sstable.metadata.comparator, indexEntry, true);
}
- protected void init() throws IOException
- {
- // This is actually a no-op, because if we call hasNext without having called setForSlice, then ReverseReader.hasNextInternal
- // will call setForSlice(Slice.ALL) which does the right thing.
- }
-
@Override
public void setForSlice(Slice slice) throws IOException
{
this.slice = slice;
- isInit = true;
// if our previous slicing already got us past the beginning of the sstable, we're done
if (indexState.isDone())
http://git-wip-us.apache.org/repos/asf/cassandra/blob/8bc567b3/test/unit/org/apache/cassandra/cql3/QueryWithIndexedSSTableTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/QueryWithIndexedSSTableTest.java b/test/unit/org/apache/cassandra/cql3/QueryWithIndexedSSTableTest.java
new file mode 100644
index 0000000..4838392
--- /dev/null
+++ b/test/unit/org/apache/cassandra/cql3/QueryWithIndexedSSTableTest.java
@@ -0,0 +1,84 @@
+/*
+ * 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.cql3;
+
+import java.util.Random;
+
+import org.junit.Test;
+
+import org.apache.cassandra.Util;
+import org.apache.cassandra.db.DecoratedKey;
+import org.apache.cassandra.db.RowIndexEntry;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
+import org.apache.cassandra.utils.ByteBufferUtil;
+
+public class QueryWithIndexedSSTableTest extends CQLTester
+{
+ @Test
+ public void queryIndexedSSTableTest() throws Throwable
+ {
+ // That test reproduces the bug from CASSANDRA-10903 and the fact we have a static column is
+ // relevant to that reproduction in particular as it forces a slightly different code path that
+ // if there wasn't a static.
+
+ int ROWS = 1000;
+ int VALUE_LENGTH = 100;
+
+ createTable("CREATE TABLE %s (k int, t int, s text static, v text, PRIMARY KEY (k, t))");
+
+ // We create a partition that is big enough that the underlying sstable will be indexed
+ // For that, we use a large-ish number of row, and a value that isn't too small.
+ String text = makeRandomTest(VALUE_LENGTH);
+ for (int i = 0; i < ROWS; i++)
+ execute("INSERT INTO %s(k, t, v) VALUES (?, ?, ?)", 0, i, text + i);
+
+ flush();
+ compact();
+
+ // Sanity check that we're testing what we want to test, that is that we're reading from an indexed
+ // sstable. Note that we'll almost surely have a single indexed sstable in practice, but it's theorically
+ // possible for a compact strategy to yield more than that and as long as one is indexed we're pretty
+ // much testing what we want. If this check ever fails on some specific setting, we'll have to either
+ // tweak ROWS and VALUE_LENGTH, or skip the test on those settings.
+ DecoratedKey dk = Util.dk(ByteBufferUtil.bytes(0));
+ boolean hasIndexed = false;
+ for (SSTableReader sstable : getCurrentColumnFamilyStore().getLiveSSTables())
+ {
+ RowIndexEntry indexEntry = sstable.getPosition(dk, SSTableReader.Operator.EQ);
+ hasIndexed |= indexEntry != null && indexEntry.isIndexed();
+ }
+ assert hasIndexed;
+
+ assertRowCount(execute("SELECT s FROM %s WHERE k = ?", 0), ROWS);
+ assertRowCount(execute("SELECT s FROM %s WHERE k = ? ORDER BY t DESC", 0), ROWS);
+
+ assertRowCount(execute("SELECT DISTINCT s FROM %s WHERE k = ?", 0), 1);
+ assertRowCount(execute("SELECT DISTINCT s FROM %s WHERE k = ? ORDER BY t DESC", 0), 1);
+ }
+
+ // Creates a random string
+ public static String makeRandomTest(int length)
+ {
+ Random random = new Random();
+ char[] chars = new char[26];
+ int i = 0;
+ for (char c = 'a'; c <= 'z'; c++)
+ chars[i++] = c;
+ return new String(chars);
+ }
+}
[3/3] cassandra git commit: Merge branch 'cassandra-3.0' into trunk
Posted by sl...@apache.org.
Merge branch 'cassandra-3.0' into trunk
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/04a99ab8
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/04a99ab8
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/04a99ab8
Branch: refs/heads/trunk
Commit: 04a99ab84cd24c10b2220667667651afb3dd29bf
Parents: e5b6a9c 8bc567b
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Dec 23 15:16:05 2015 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Dec 23 15:16:05 2015 +0100
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../columniterator/AbstractSSTableIterator.java | 37 ++++-----
.../db/columniterator/SSTableIterator.java | 38 +++------
.../columniterator/SSTableReversedIterator.java | 28 ++-----
.../cql3/QueryWithIndexedSSTableTest.java | 84 ++++++++++++++++++++
5 files changed, 121 insertions(+), 67 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/04a99ab8/CHANGES.txt
----------------------------------------------------------------------
diff --cc CHANGES.txt
index e36e4c2,a669b17..ab6cb92
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@@ -1,28 -1,5 +1,29 @@@
-3.0.3
+3.2
+ * Add forceUserDefinedCleanup to allow more flexible cleanup (CASSANDRA-10708)
+ * (cqlsh) allow setting TTL with COPY (CASSANDRA-9494)
+ * Fix EstimatedHistogram creation in nodetool tablehistograms (CASSANDRA-10859)
+ * Establish bootstrap stream sessions sequentially (CASSANDRA-6992)
+ * Sort compactionhistory output by timestamp (CASSANDRA-10464)
+ * More efficient BTree removal (CASSANDRA-9991)
+ * Make tablehistograms accept the same syntax as tablestats (CASSANDRA-10149)
+ * Group pending compactions based on table (CASSANDRA-10718)
+ * Add compressor name in sstablemetadata output (CASSANDRA-9879)
+ * Fix type casting for counter columns (CASSANDRA-10824)
+ * Prevent running Cassandra as root (CASSANDRA-8142)
+ * bound maximum in-flight commit log replay mutation bytes to 64 megabytes (CASSANDRA-8639)
+ * Normalize all scripts (CASSANDRA-10679)
+ * Make compression ratio much more accurate (CASSANDRA-10225)
+ * Optimize building of Clustering object when only one is created (CASSANDRA-10409)
+ * Make index building pluggable (CASSANDRA-10681)
+ * Add sstable flush observer (CASSANDRA-10678)
+ * Improve NTS endpoints calculation (CASSANDRA-10200)
+ * Improve performance of the folderSize function (CASSANDRA-10677)
+ * Add support for type casting in selection clause (CASSANDRA-10310)
+ * Added graphing option to cassandra-stress (CASSANDRA-7918)
+ * Abort in-progress queries that time out (CASSANDRA-7392)
+ * Add transparent data encryption core classes (CASSANDRA-9945)
+Merged from 3.0:
+ * Fix potential assertion error when reading static columns (CASSANDRA-0903)
* Avoid NoSuchElementException when executing empty batch (CASSANDRA-10711)
* Avoid building PartitionUpdate in toString (CASSANDRA-10897)
* Reduce heap spent when receiving many SSTables (CASSANDRA-10797)