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 2013/11/13 18:02:00 UTC
[1/2] git commit: Fix paging with reversed slices
Updated Branches:
refs/heads/trunk 0619da2ae -> cb871ba90
Fix paging with reversed slices
patch by slebresne; reviewed by iamalesksey for CASSANDRA-6343
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/5008507c
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/5008507c
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/5008507c
Branch: refs/heads/trunk
Commit: 5008507ca21265e0c6be53d61024baa7eaf187fc
Parents: b457156
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Nov 13 17:59:07 2013 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Nov 13 17:59:07 2013 +0100
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../org/apache/cassandra/db/ColumnFamily.java | 5 ++
.../service/pager/AbstractQueryPager.java | 49 ++++++++++++++------
.../service/pager/RangeNamesQueryPager.java | 5 ++
.../service/pager/RangeSliceQueryPager.java | 9 +++-
.../service/pager/SliceQueryPager.java | 9 +++-
.../cassandra/service/QueryPagerTest.java | 32 ++++++++++++-
7 files changed, 92 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5008507c/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index e4f1862..159e8de 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -20,6 +20,7 @@
* Fix serialization bug in PagedRange with 2ndary indexes (CASSANDRA-6299)
* Fix CQL3 table validation in Thrift (CASSANDRA-6140)
* Fix bug missing results with IN clauses (CASSANDRA-6327)
+ * Fix paging with reversed slices (CASSANDRA-6343)
Merged from 1.2:
* add non-jamm path for cached statements (CASSANDRA-6293)
* (Hadoop) Require CFRR batchSize to be at least 2 (CASSANDRA-6114)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5008507c/src/java/org/apache/cassandra/db/ColumnFamily.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/ColumnFamily.java b/src/java/org/apache/cassandra/db/ColumnFamily.java
index b2c5ac4..47b14b9 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamily.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamily.java
@@ -451,6 +451,11 @@ public abstract class ColumnFamily implements Iterable<Column>, IRowCacheEntry
return getSortedColumns().iterator();
}
+ public Iterator<Column> reverseIterator()
+ {
+ return getReverseSortedColumns().iterator();
+ }
+
public boolean hasIrrelevantData(int gcBefore)
{
// Do we have gcable deletion infos?
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5008507c/src/java/org/apache/cassandra/service/pager/AbstractQueryPager.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/service/pager/AbstractQueryPager.java b/src/java/org/apache/cassandra/service/pager/AbstractQueryPager.java
index 62cd454..d040203 100644
--- a/src/java/org/apache/cassandra/service/pager/AbstractQueryPager.java
+++ b/src/java/org/apache/cassandra/service/pager/AbstractQueryPager.java
@@ -93,7 +93,7 @@ abstract class AbstractQueryPager implements QueryPager
remaining++;
}
// Otherwise, if 'lastWasRecorded', we queried for one more than the page size,
- // so if the page was is full, trim the last entry
+ // so if the page is full, trim the last entry
else if (lastWasRecorded && !exhausted)
{
// We've asked for one more than necessary
@@ -161,11 +161,14 @@ abstract class AbstractQueryPager implements QueryPager
protected abstract List<Row> queryNextPage(int pageSize, ConsistencyLevel consistency, boolean localQuery) throws RequestValidationException, RequestExecutionException;
protected abstract boolean containsPreviousLast(Row first);
protected abstract boolean recordLast(Row last);
+ protected abstract boolean isReversed();
private List<Row> discardFirst(List<Row> rows)
{
Row first = rows.get(0);
- ColumnFamily newCf = discardFirst(first.cf);
+ ColumnFamily newCf = isReversed()
+ ? discardLast(first.cf)
+ : discardFirst(first.cf);
int count = newCf.getColumnCount();
List<Row> newRows = new ArrayList<Row>(count == 0 ? rows.size() - 1 : rows.size());
@@ -179,7 +182,9 @@ abstract class AbstractQueryPager implements QueryPager
private List<Row> discardLast(List<Row> rows)
{
Row last = rows.get(rows.size() - 1);
- ColumnFamily newCf = discardLast(last.cf);
+ ColumnFamily newCf = isReversed()
+ ? discardFirst(last.cf)
+ : discardLast(last.cf);
int count = newCf.getColumnCount();
List<Row> newRows = new ArrayList<Row>(count == 0 ? rows.size() - 1 : rows.size());
@@ -200,11 +205,27 @@ abstract class AbstractQueryPager implements QueryPager
private ColumnFamily discardFirst(ColumnFamily cf)
{
+ boolean isReversed = isReversed();
+ DeletionInfo.InOrderTester tester = cf.deletionInfo().inOrderTester(isReversed);
+ return isReversed
+ ? discardTail(cf, cf.reverseIterator(), tester)
+ : discardHead(cf, cf.iterator(), tester);
+ }
+
+ private ColumnFamily discardLast(ColumnFamily cf)
+ {
+ boolean isReversed = isReversed();
+ DeletionInfo.InOrderTester tester = cf.deletionInfo().inOrderTester(isReversed);
+ return isReversed
+ ? discardHead(cf, cf.reverseIterator(), tester)
+ : discardTail(cf, cf.iterator(), tester);
+ }
+
+ private ColumnFamily discardHead(ColumnFamily cf, Iterator<Column> iter, DeletionInfo.InOrderTester tester)
+ {
ColumnFamily copy = cf.cloneMeShallow();
ColumnCounter counter = columnCounter();
- Iterator<Column> iter = cf.iterator();
- DeletionInfo.InOrderTester tester = cf.inOrderDeletionTester();
// Discard the first live
while (iter.hasNext())
{
@@ -220,22 +241,24 @@ abstract class AbstractQueryPager implements QueryPager
return copy;
}
- private ColumnFamily discardLast(ColumnFamily cf)
+ private ColumnFamily discardTail(ColumnFamily cf, Iterator<Column> iter, DeletionInfo.InOrderTester tester)
{
ColumnFamily copy = cf.cloneMeShallow();
- // Redoing the counting like that is not extremely efficient, but
- // discardLast is only called in case of a race between paging and
- // a deletion, which is pretty unlikely, so probably not a big deal
+ // Redoing the counting like that is not extremely efficient.
+ // This is called only for reversed slices or in the case of a race between
+ // paging and a deletion (pretty unlikely), so this is probably acceptable.
int liveCount = columnCounter().countAll(cf).live();
ColumnCounter counter = columnCounter();
- DeletionInfo.InOrderTester tester = cf.inOrderDeletionTester();
// Discard the first live
- for (Column c : cf)
+ while (iter.hasNext())
{
+ Column c = iter.next();
counter.count(c, tester);
- if (counter.live() < liveCount)
- copy.addColumn(c);
+ if (counter.live() >= liveCount)
+ break;
+
+ copy.addColumn(c);
}
return copy;
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5008507c/src/java/org/apache/cassandra/service/pager/RangeNamesQueryPager.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/service/pager/RangeNamesQueryPager.java b/src/java/org/apache/cassandra/service/pager/RangeNamesQueryPager.java
index 57fb05b..e3b0cf8 100644
--- a/src/java/org/apache/cassandra/service/pager/RangeNamesQueryPager.java
+++ b/src/java/org/apache/cassandra/service/pager/RangeNamesQueryPager.java
@@ -91,6 +91,11 @@ public class RangeNamesQueryPager extends AbstractQueryPager
return false;
}
+ protected boolean isReversed()
+ {
+ return false;
+ }
+
private AbstractBounds<RowPosition> makeExcludingKeyBounds(RowPosition lastReturnedKey)
{
// We return a range that always exclude lastReturnedKey, since we've already
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5008507c/src/java/org/apache/cassandra/service/pager/RangeSliceQueryPager.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/service/pager/RangeSliceQueryPager.java b/src/java/org/apache/cassandra/service/pager/RangeSliceQueryPager.java
index 42a9585..1f4ba78 100644
--- a/src/java/org/apache/cassandra/service/pager/RangeSliceQueryPager.java
+++ b/src/java/org/apache/cassandra/service/pager/RangeSliceQueryPager.java
@@ -91,16 +91,21 @@ public class RangeSliceQueryPager extends AbstractQueryPager
{
return lastReturnedKey != null
&& lastReturnedKey.equals(first.key)
- && lastReturnedName.equals(firstName(first.cf));
+ && lastReturnedName.equals(isReversed() ? lastName(first.cf) : firstName(first.cf));
}
protected boolean recordLast(Row last)
{
lastReturnedKey = last.key;
- lastReturnedName = lastName(last.cf);
+ lastReturnedName = isReversed() ? firstName(last.cf) : lastName(last.cf);
return true;
}
+ protected boolean isReversed()
+ {
+ return ((SliceQueryFilter)command.predicate).reversed;
+ }
+
private AbstractBounds<RowPosition> makeIncludingKeyBounds(RowPosition lastReturnedKey)
{
// We always include lastReturnedKey since we may still be paging within a row,
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5008507c/src/java/org/apache/cassandra/service/pager/SliceQueryPager.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/service/pager/SliceQueryPager.java b/src/java/org/apache/cassandra/service/pager/SliceQueryPager.java
index 1d77144..e3825a9 100644
--- a/src/java/org/apache/cassandra/service/pager/SliceQueryPager.java
+++ b/src/java/org/apache/cassandra/service/pager/SliceQueryPager.java
@@ -76,12 +76,17 @@ public class SliceQueryPager extends AbstractQueryPager implements SinglePartiti
protected boolean containsPreviousLast(Row first)
{
- return lastReturned != null && lastReturned.equals(firstName(first.cf));
+ return lastReturned != null && lastReturned.equals(isReversed() ? lastName(first.cf) : firstName(first.cf));
}
protected boolean recordLast(Row last)
{
- lastReturned = lastName(last.cf);
+ lastReturned = isReversed() ? firstName(last.cf) : lastName(last.cf);
return true;
}
+
+ protected boolean isReversed()
+ {
+ return command.filter.reversed;
+ }
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5008507c/test/unit/org/apache/cassandra/service/QueryPagerTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/service/QueryPagerTest.java b/test/unit/org/apache/cassandra/service/QueryPagerTest.java
index 3fc2ac2..f395cf4 100644
--- a/test/unit/org/apache/cassandra/service/QueryPagerTest.java
+++ b/test/unit/org/apache/cassandra/service/QueryPagerTest.java
@@ -117,7 +117,12 @@ public class QueryPagerTest extends SchemaLoader
private static ReadCommand sliceQuery(String key, String start, String end, int count)
{
- SliceQueryFilter filter = new SliceQueryFilter(bytes(start), bytes(end), false, count);
+ return sliceQuery(key, start, end, false, count);
+ }
+
+ private static ReadCommand sliceQuery(String key, String start, String end, boolean reversed, int count)
+ {
+ SliceQueryFilter filter = new SliceQueryFilter(bytes(start), bytes(end), reversed, count);
// Note: for MultiQueryTest, we need the same timestamp/expireBefore for all queries, so we just use 0 as it doesn't matter here.
return new SliceFromReadCommand(KS, bytes(key), CF, 0, filter);
}
@@ -188,6 +193,31 @@ public class QueryPagerTest extends SchemaLoader
}
@Test
+ public void reversedSliceQueryTest() throws Exception
+ {
+ QueryPager pager = QueryPagers.localPager(sliceQuery("k0", "c8", "c1", true, 10));
+
+ List<Row> page;
+
+ assertFalse(pager.isExhausted());
+ page = pager.fetchPage(3);
+ assertEquals(toString(page), 1, page.size());
+ assertRow(page.get(0), "k0", "c6", "c7", "c8");
+
+ assertFalse(pager.isExhausted());
+ page = pager.fetchPage(3);
+ assertEquals(toString(page), 1, page.size());
+ assertRow(page.get(0), "k0", "c3", "c4", "c5");
+
+ assertFalse(pager.isExhausted());
+ page = pager.fetchPage(3);
+ assertEquals(toString(page), 1, page.size());
+ assertRow(page.get(0), "k0", "c1", "c2");
+
+ assertTrue(pager.isExhausted());
+ }
+
+ @Test
public void MultiQueryTest() throws Exception
{
QueryPager pager = QueryPagers.localPager(new Pageable.ReadCommands(new ArrayList<ReadCommand>() {{
[2/2] git commit: Merge branch 'cassandra-2.0' into trunk
Posted by sl...@apache.org.
Merge branch 'cassandra-2.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/cb871ba9
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/cb871ba9
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/cb871ba9
Branch: refs/heads/trunk
Commit: cb871ba908dac0e8773a50dc805b272ac2c794d1
Parents: 0619da2 5008507
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Nov 13 18:01:51 2013 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Wed Nov 13 18:01:51 2013 +0100
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../org/apache/cassandra/db/ColumnFamily.java | 5 ++
.../service/pager/AbstractQueryPager.java | 49 ++++++++++++++------
.../service/pager/RangeNamesQueryPager.java | 5 ++
.../service/pager/RangeSliceQueryPager.java | 9 +++-
.../service/pager/SliceQueryPager.java | 9 +++-
.../cassandra/service/QueryPagerTest.java | 32 ++++++++++++-
7 files changed, 92 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/cb871ba9/CHANGES.txt
----------------------------------------------------------------------