Cassandra
  1. Cassandra
  2. CASSANDRA-126

Incorrect IOException in SequenceFile when BF returns false positive

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      In SequenceFile.AbstractReader.seekTo(), an IOException is thrown when a key can't be found in an SSTable. This is incorrect. The BF associated with the SSTable could return a false positive. So a key may indeed not be present in the SSTable file. We need to handle this case accordingly.

      1. 126-v2.patch
        4 kB
        Jonathan Ellis
      2. issue126.patch_v1.test
        2 kB
        Jun Rao
      3. issue126.patch_v1
        3 kB
        Jun Rao

        Activity

        Jun Rao created issue -
        Hide
        Jun Rao added a comment -

        Attach a fix that deals with non-existing keys in SSTables explicitly.

        Show
        Jun Rao added a comment - Attach a fix that deals with non-existing keys in SSTables explicitly.
        Jun Rao made changes -
        Field Original Value New Value
        Attachment issue126.patch_v1 [ 12406957 ]
        Hide
        Jonathan Ellis added a comment -

        If I understand what you are doing, shouldn't you take out the IOException catches that deal with the current way of doing things? e.g. in SSTable,

        catch( IOException ex )

        { logger_.info("Bloom filter false positive", ex); }

        I'd also like to see a test case that exercises this code path.

        Show
        Jonathan Ellis added a comment - If I understand what you are doing, shouldn't you take out the IOException catches that deal with the current way of doing things? e.g. in SSTable, catch( IOException ex ) { logger_.info("Bloom filter false positive", ex); } I'd also like to see a test case that exercises this code path.
        Hide
        Jonathan Ellis added a comment -

        sorry, I was looking at the wrong git repo. I don't see any exceptions anymore that you'd want to change, either.

        Show
        Jonathan Ellis added a comment - sorry, I was looking at the wrong git repo. I don't see any exceptions anymore that you'd want to change, either.
        Hide
        Jun Rao added a comment -

        Add a testcase that covers the code path when a BF gives a false positive.

        Show
        Jun Rao added a comment - Add a testcase that covers the code path when a BF gives a false positive.
        Jun Rao made changes -
        Attachment issue126.patch_v1.test [ 12406962 ]
        Hide
        Jonathan Ellis added a comment -

        thanks, looks good.

        i'm not comfortable w/ introducing more code duplication, though. what do you think of this version?

        Show
        Jonathan Ellis added a comment - thanks, looks good. i'm not comfortable w/ introducing more code duplication, though. what do you think of this version?
        Jonathan Ellis made changes -
        Attachment 126-v2.patch [ 12406972 ]
        Hide
        Jun Rao added a comment -

        patch v2 looks good to me.

        Show
        Jun Rao added a comment - patch v2 looks good to me.
        Hide
        Jonathan Ellis added a comment -

        committed

        Show
        Jonathan Ellis added a comment - committed
        Jonathan Ellis made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        Integrated in Cassandra #55 (See http://hudson.zones.apache.org/hudson/job/Cassandra/55/)
        avoid raising an exception in SequenceFile.next when a key does not exist (e.g. when bloom filter gives a false positive). patch by Jun Rao; reviewed by jbellis for

        Show
        Hudson added a comment - Integrated in Cassandra #55 (See http://hudson.zones.apache.org/hudson/job/Cassandra/55/ ) avoid raising an exception in SequenceFile.next when a key does not exist (e.g. when bloom filter gives a false positive). patch by Jun Rao; reviewed by jbellis for
        Gavin made changes -
        Workflow no-reopen-closed, patch-avail [ 12462337 ] patch-available, re-open possible [ 12750299 ]
        Gavin made changes -
        Workflow patch-available, re-open possible [ 12750299 ] reopen-resolved, no closed status, patch-avail, testing [ 12756702 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        5h 42m 1 Jonathan Ellis 01/May/09 03:25

          People

          • Assignee:
            Jun Rao
            Reporter:
            Jun Rao
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development