Return-Path: 
 <commits-return-35626-apmail-cassandra-commits-archive=cassandra.apache.org@cassandra.apache.org>
X-Original-To: apmail-cassandra-commits-archive@www.apache.org
Delivered-To: apmail-cassandra-commits-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 596F04FB7
	for <apmail-cassandra-commits-archive@www.apache.org>;
 Wed, 29 Jun 2011 15:15:53 +0000 (UTC)
Received: (qmail 68456 invoked by uid 500); 29 Jun 2011 15:15:53 -0000
Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org
Received: (qmail 68362 invoked by uid 500); 29 Jun 2011 15:15:52 -0000
Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:commits-help@cassandra.apache.org>
List-Unsubscribe: <mailto:commits-unsubscribe@cassandra.apache.org>
List-Post: <mailto:commits@cassandra.apache.org>
List-Id: <commits.cassandra.apache.org>
Reply-To: dev@cassandra.apache.org
Delivered-To: mailing list commits@cassandra.apache.org
Received: (qmail 68354 invoked by uid 99); 29 Jun 2011 15:15:52 -0000
Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 29 Jun 2011 15:15:52 +0000
X-ASF-Spam-Status: No, hits=-2000.0 required=5.0
	tests=ALL_TRUSTED
X-Spam-Check-By: apache.org
Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 29 Jun 2011 15:15:50 +0000
Received: from eris.apache.org (localhost [127.0.0.1])
	by eris.apache.org (Postfix) with ESMTP id C49F823888CF
	for <commits@cassandra.apache.org>; Wed, 29 Jun 2011 15:15:30 +0000 (UTC)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1141129 - in /cassandra/branches/cassandra-0.7:
 CHANGES.txt
 src/java/org/apache/cassandra/db/ColumnFamilyStore.java
Date: Wed, 29 Jun 2011 15:15:30 -0000
To: commits@cassandra.apache.org
From: slebresne@apache.org
X-Mailer: svnmailer-1.0.8
Message-Id: <20110629151530.C49F823888CF@eris.apache.org>

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)