Author: slebresne Date: Wed Jun 29 15:15:29 2011 New Revision: 1141129 URL: http://svn.apache.org/viewvc?rev=1141129&view=rev Log: Fix scan wrongly throwing assertion errors patch by slebresne; reviewed by jbellis for CASSANDRA-2653
Modified: cassandra/branches/cassandra-0.7/CHANGES.txt cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ColumnFamilyStore.java Modified: cassandra/branches/cassandra-0.7/CHANGES.txt URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/CHANGES.txt?rev=1141129&r1=1141128&r2=1141129&view=diff ============================================================================== --- cassandra/branches/cassandra-0.7/CHANGES.txt (original) +++ cassandra/branches/cassandra-0.7/CHANGES.txt Wed Jun 29 15:15:29 2011 @@ -26,6 +26,7 @@ * fix race that could result in Hadoop writer failing to throw an exception encountered after close() (CASSANDRA-2755) * fix CLI parsing of read_repair_chance (CASSANDRA-2837) + * fix scan wrongly throwing assertion error (CASSANDRA-2653) 0.7.6 Modified: cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ColumnFamilyStore.java URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ColumnFamilyStore.java?rev=1141129&r1=1141128&r2=1141129&view=diff ============================================================================== --- cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ColumnFamilyStore.java (original) +++ cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ColumnFamilyStore.java Wed Jun 29 15:15:29 2011 @@ -1486,6 +1486,16 @@ public class ColumnFamilyStore implement return rows; } + private NamesQueryFilter getExtraFilter(IndexClause clause) + { + SortedSet<ByteBuffer> columns = new TreeSet<ByteBuffer>(getComparator()); + for (IndexExpression expr : clause.expressions) + { + columns.add(expr.column_name); + } + return new NamesQueryFilter(columns); + } + public List<Row> scan(IndexClause clause, AbstractBounds range, IFilter dataFilter) { // Start with the most-restrictive indexed clause, then apply remaining clauses @@ -1502,50 +1512,33 @@ public class ColumnFamilyStore implement // it needs to be expanded to include those too IFilter firstFilter = dataFilter; NamesQueryFilter extraFilter = null; - if (clause.expressions.size() > 1) + if (dataFilter instanceof SliceQueryFilter) { - if (dataFilter instanceof SliceQueryFilter) + // if we have a high chance of getting all the columns in a single index slice, do that. + // otherwise, we'll create an extraFilter (lazily) to fetch by name the columns referenced by the additional expressions. + if (getMaxRowSize() < DatabaseDescriptor.getColumnIndexSize()) { - // if we have a high chance of getting all the columns in a single index slice, do that. - // otherwise, create an extraFilter to fetch by name the columns referenced by the additional expressions. - if (getMaxRowSize() < DatabaseDescriptor.getColumnIndexSize()) - { - logger.debug("Expanding slice filter to entire row to cover additional expressions"); - firstFilter = new SliceQueryFilter(ByteBufferUtil.EMPTY_BYTE_BUFFER, - ByteBufferUtil.EMPTY_BYTE_BUFFER, - ((SliceQueryFilter) dataFilter).reversed, - Integer.MAX_VALUE); - } - else - { - logger.debug("adding extraFilter to cover additional expressions"); - SortedSet<ByteBuffer> columns = new TreeSet<ByteBuffer>(getComparator()); - for (IndexExpression expr : clause.expressions) - { - if (expr == primary) - continue; - columns.add(expr.column_name); - } - extraFilter = new NamesQueryFilter(columns); - } + logger.debug("Expanding slice filter to entire row to cover additional expressions"); + firstFilter = new SliceQueryFilter(ByteBufferUtil.EMPTY_BYTE_BUFFER, + ByteBufferUtil.EMPTY_BYTE_BUFFER, + ((SliceQueryFilter) dataFilter).reversed, + Integer.MAX_VALUE); } - else + } + else + { + logger.debug("adding columns to firstFilter to cover additional expressions"); + // just add in columns that are not part of the resultset + assert dataFilter instanceof NamesQueryFilter; + SortedSet<ByteBuffer> columns = new TreeSet<ByteBuffer>(getComparator()); + for (IndexExpression expr : clause.expressions) { - logger.debug("adding columns to firstFilter to cover additional expressions"); - // just add in columns that are not part of the resultset - assert dataFilter instanceof NamesQueryFilter; - SortedSet<ByteBuffer> columns = new TreeSet<ByteBuffer>(getComparator()); - for (IndexExpression expr : clause.expressions) - { - if (expr == primary || ((NamesQueryFilter) dataFilter).columns.contains(expr.column_name)) - continue; - columns.add(expr.column_name); - } - if (columns.size() > 0) - { - columns.addAll(((NamesQueryFilter) dataFilter).columns); - firstFilter = new NamesQueryFilter(columns); - } + columns.add(expr.column_name); + } + if (columns.size() > 0) + { + columns.addAll(((NamesQueryFilter) dataFilter).columns); + firstFilter = new NamesQueryFilter(columns); } } @@ -1600,18 +1593,23 @@ public class ColumnFamilyStore implement // get the row columns requested, and additional columns for the expressions if necessary ColumnFamily data = getColumnFamily(new QueryFilter(dk, path, firstFilter)); - assert data != null : String.format("No data found for %s in %s:%s (original filter %s) from expression %s", - firstFilter, dk, path, dataFilter, expressionString(primary)); + // While we the column family we'll get in the end should contains the primary clause column, the firstFilter may not have found it. + if (data == null) + data = ColumnFamily.create(metadata); logger.debug("fetched data row {}", data); - if (extraFilter != null) + if (dataFilter instanceof SliceQueryFilter) { // we might have gotten the expression columns in with the main data slice, but // we can't know for sure until that slice is done. So, we'll do the extra query // if we go through and any expression columns are not present. for (IndexExpression expr : clause.expressions) { - if (expr != primary && data.getColumn(expr.column_name) == null) + if (data.getColumn(expr.column_name) == null) { + logger.debug("adding extraFilter to cover additional expressions"); + // Lazily creating extra filter + if (extraFilter == null) + extraFilter = getExtraFilter(clause); data.addAll(getColumnFamily(new QueryFilter(dk, path, extraFilter))); break; } @@ -1675,11 +1673,10 @@ public class ColumnFamilyStore implement private static boolean satisfies(ColumnFamily data, IndexClause clause, IndexExpression first) { + // We enforces even the primary clause because reads are not synchronized with writes and it is thus possible to have a race + // where the index returned a row which doesn't have the primarycolumn when we actually read it for (IndexExpression expression : clause.expressions) { - // (we can skip "first" since we already know it's satisfied) - if (expression == first) - continue; // check column data vs expression IColumn column = data.getColumn(expression.column_name); if (column == null)