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. issue126.patch_v1
        3 kB
        Jun Rao
      2. issue126.patch_v1.test
        2 kB
        Jun Rao
      3. 126-v2.patch
        4 kB
        Jonathan Ellis

        Activity

        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
        Hide
        Jonathan Ellis added a comment -

        committed

        Show
        Jonathan Ellis added a comment - committed
        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 -

        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?
        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.
        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
        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
        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.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development