AllRowsQuery does not respect forTokenRange() when using an iterator #258

Closed
senecaso-rt opened this Issue Mar 29, 2013 · 1 comment

Comments

Projects
None yet
2 participants
Contributor

senecaso-rt commented Mar 29, 2013

Is there a reason why the iterator in ThriftAllRowsImpl.iterator() is not using the start/end token as they are defined in the query object? Instead, these values are hardcoded to either "0" or the partitioner's min/max token value (depending on the version of the source you look at). To demonstrate the issue, below is a snippet that can be added to the ThriftKeyspaceAllRowsTest.java test.

    @Test
    public void testTokenRangeTest() {
        try {
            OperationResult<Rows<Long, String>> rows = keyspace.prepareQuery(CF_ALL_ROWS)
                    .getAllRows()
                    .setRowLimit(5)
                    .setExceptionCallback(new ExceptionCallback() {
                        @Override
                        public boolean onException(ConnectionException e) {
                            Assert.fail(e.getMessage());
                            return true;
                        }
                    })
                    .forTokenRange("9452287970026068429538183539771339207", "37809151880104273718152734159085356828")
                    .execute();

            Iterator<Row<Long, String>> itr = rows.getResult().iterator();

            while (itr.hasNext()) {
                Row<Long, String> row = itr.next();
                LOG.info("Row: " + row.getKey() + " count=" + row.getColumns().size());
            }

            Set<Long> set = getKeySet(rows.getResult());
            LOG.info(set.toString());
            // only a subset of the rows should have been returned
            Assert.assertEquals(4,  set.size());
        } catch (ConnectionException e) {
            Assert.fail();
        }
    }

As far as I can tell, ThriftAllRowsImpl.iterator() should simply use the specified start/end tokens, and default to the min/max/"0" if none has been specified. Is this a bug, or am I doing something wrong? I know I can use executeWithCallback(), but an Iterator makes my life a lot easier and I see no reason why this wouldn't work.

    @Override
    public Iterator<Row<K, C>> iterator() {
        return new Iterator<Row<K, C>>() {
            private KeyRange range;
            private org.apache.cassandra.thrift.KeySlice lastRow;
            private List<org.apache.cassandra.thrift.KeySlice> list = null;
            private Iterator<org.apache.cassandra.thrift.KeySlice> iter = null;
            private boolean bContinueSearch = true;
            private boolean bIgnoreTombstones = true;

            {
                String startToken = query.getStartToken() == null ? "0" : query.getStartToken().toString();
                String endToken = query.getEndToken() == null ? "0" : query.getEndToken().toString();
                range = new KeyRange().setCount(query.getBlockSize()).setStart_token(startToken).setEnd_token(endToken);
...
            }
...
}
Contributor

senecaso-rt commented Apr 1, 2013

I have added a fix for this in Pull Request 261 (#261)

elandau closed this Apr 1, 2013

@omar-eyeviewdigital omar-eyeviewdigital pushed a commit to omar-eyeviewdigital/astyanax that referenced this issue Oct 31, 2013

@senecaso-rt senecaso-rt ISSUE 258 (Netflix#258):
-if the user has specified a token range to be used in the AllRowsQuery and then requests an iterator, respect the token range requested instead of always iterating over all possible rows
a77b6c5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment