cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [1/2] git commit: Fix paging with reversed slices
Date Wed, 13 Nov 2013 17:02:00 GMT
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 <sylvain@datastax.com>
Authored: Wed Nov 13 17:59:07 2013 +0100
Committer: Sylvain Lebresne <sylvain@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>()
{{


Mime
View raw message