[ 
https://issues.apache.org/jira/browse/CASSANDRA-5487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13634609#comment-13634609
 ] 

Jonathan Ellis edited comment on CASSANDRA-5487 at 4/17/13 11:37 PM:
---------------------------------------------------------------------

Patch attached that adds descriptor version "ic" to fix the problem.

All unit tests pass except RangeTombstoneTest.  I'm pretty baffled by this.  I 
have narrowed it down thus far:
# If I revert just SSTableNamesIterator, the test fails in the same place
# If I force SSTNI to take the readSimpleColumns path, the test passes

Apparently another change is breaking something in the indexed path as a side 
effect but damned if I can see it.  Throwing this up in case I'm missing 
something obvious that another set of eyes would see.

Some notes:

- IndexedEntry.serializedSize was missing the 8 bytes for the position.  Fixed 
by pulling out promotedSize method.
- Removed RowIndexEntry.isIndexed since it was difficult to tell whether 
"hasPromotedIndexes" or "contains enough columns to be indexed" was meant.  
(This is probably what caused the bug here.)
- Updated RIE to return sane values when there are no columns instead of 
UnsupportedOperation.  This allows the code calling it to deal with fewer 
special cases.
- I've renamed RIE instances to rowEntry which caused a bit of churn in the 
patch, sorry.  (Easy to confuse with IndexInfo when the code deals with both, 
otherwise.)  I can vouch that intellij diff handles it pretty well. :)
- I'm pretty sure that checking the column bloom filter at all would be buggy, 
since that way we could miss range tombstones.  I've removed the BF-filtering 
code from SSTNI but have not done a full backport of CASSANDRA-4885.
                
      was (Author: jbellis):
    Patch attached that adds descriptor version "ic" to fix the problem.

All unit tests pass except RangeTombstoneTest.  I'm pretty baffled by this.  I 
have narrowed it down thus far:
# If I revert just SSTableNamesIterator, the test fails in the same place
# If I force SSTNI to take the readSimpleColumns path, the test passes

Apparently another change is breaking this as a side effect but damned if I can 
see it.  Throwing this up in case I'm missing something obvious that another 
set of eyes would see.

Some notes:

- IndexedEntry.serializedSize was missing the 8 bytes for the position.  Fixed 
by pulling out promotedSize method.
- Removed RowIndexEntry.isIndexed since it was difficult to tell whether 
"hasPromotedIndexes" or "contains enough columns to be indexed" was meant.  
(This is probably what caused the bug here.)
- Updated RIE to return sane values when there are no columns instead of 
UnsupportedOperation.  This allows the code calling it to deal with fewer 
special cases.
- I've renamed RIE instances to rowEntry which caused a bit of churn in the 
patch, sorry.  (Easy to confuse with IndexInfo when the code deals with both, 
otherwise.)  I can vouch that intellij diff handles it pretty well. :)
- I'm pretty sure that checking the column bloom filter at all would be buggy, 
since that way we could miss range tombstones.  I've removed the BF-filtering 
code from SSTNI but have not done a full backport of CASSANDRA-4885.
                  
> Promote row-level tombstones to index file
> ------------------------------------------
>
>                 Key: CASSANDRA-5487
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-5487
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core
>    Affects Versions: 1.2.0
>            Reporter: Jonathan Ellis
>             Fix For: 2.0
>
>         Attachments: 5487.txt
>
>
> The idea behind promoted indexes (CASSANDRA-2319) was we could skip a seek to 
> the row header by keeping the column index in the index file.  But, we skip 
> writing the row-level tombstone to the index file unless it also has some 
> column data.  So unless we read the tombstone from the data file (where it is 
> guaranteed to exist) we can return incorrect results.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to