Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-10505

Import.filterKv does not call Filter.filterRowKey

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.96.2, 0.94.17
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The general contract of a Filter is that filterRowKey is called before filterKeyValue.
      Import is using Filters for custom filtering but it does not called filterRowKey at all. That throws off some Filters (such as RowFilter, and more recently PrefixFilter, and InclusiveStopFilter). See HBASE-10493 and HBASE-10485.

      1. 10505-0.94.txt
        1 kB
        Lars Hofhansl
      2. 10505-0.94-v2.txt
        3 kB
        Lars Hofhansl
      3. 10505-0.96-v2.txt
        4 kB
        Lars Hofhansl

        Activity

        Hide
        lhofhansl Lars Hofhansl added a comment -

        Yeah, this is fixed in 0.98 and later.

        Show
        lhofhansl Lars Hofhansl added a comment - Yeah, this is fixed in 0.98 and later.
        Hide
        vasu.mariyala@gmail.com Vasu Mariyala added a comment -

        For branches > 0.96, the issue is fixed with HBASE-10416

        Show
        vasu.mariyala@gmail.com Vasu Mariyala added a comment - For branches > 0.96, the issue is fixed with HBASE-10416
        Hide
        apurtell Andrew Purtell added a comment -

        Why not branches > 0.96?

        Show
        apurtell Andrew Purtell added a comment - Why not branches > 0.96?
        Hide
        vasu.mariyala@gmail.com Vasu Mariyala added a comment -

        Lars Hofhansl May be this is one of the issues of HBASE-10416 ?

        I think we need to fix the TestImportExport.testWithFilter test case by inserting atleast another row. This works currently because we insert only one row ("row1"), perform export, perform import with prefix filter. The prefix filter filterRowKey is never called (before this fix) so all the rows are included in the import. But since we only have one row ("row1"), the test case doesn't really test the functionality of import with filter.

        Show
        vasu.mariyala@gmail.com Vasu Mariyala added a comment - Lars Hofhansl May be this is one of the issues of HBASE-10416 ? I think we need to fix the TestImportExport.testWithFilter test case by inserting atleast another row. This works currently because we insert only one row ("row1"), perform export, perform import with prefix filter. The prefix filter filterRowKey is never called (before this fix) so all the rows are included in the import. But since we only have one row ("row1"), the test case doesn't really test the functionality of import with filter.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in hbase-0.96-hadoop2 #201 (See https://builds.apache.org/job/hbase-0.96-hadoop2/201/)
        HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567524)

        • /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in hbase-0.96-hadoop2 #201 (See https://builds.apache.org/job/hbase-0.96-hadoop2/201/ ) HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567524) /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in hbase-0.96 #291 (See https://builds.apache.org/job/hbase-0.96/291/)
        HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567524)

        • /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in hbase-0.96 #291 (See https://builds.apache.org/job/hbase-0.96/291/ ) HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567524) /hbase/branches/0.96/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in HBase-0.94-JDK7 #47 (See https://builds.apache.org/job/HBase-0.94-JDK7/47/)
        HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567522)

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in HBase-0.94-JDK7 #47 (See https://builds.apache.org/job/HBase-0.94-JDK7/47/ ) HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567522) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in HBase-0.94 #1284 (See https://builds.apache.org/job/HBase-0.94/1284/)
        HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567522)

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in HBase-0.94 #1284 (See https://builds.apache.org/job/HBase-0.94/1284/ ) HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567522) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in HBase-0.94-on-Hadoop-2 #20 (See https://builds.apache.org/job/HBase-0.94-on-Hadoop-2/20/)
        HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567522)

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in HBase-0.94-on-Hadoop-2 #20 (See https://builds.apache.org/job/HBase-0.94-on-Hadoop-2/20/ ) HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567522) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in HBase-0.94-security #409 (See https://builds.apache.org/job/HBase-0.94-security/409/)
        HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567522)

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in HBase-0.94-security #409 (See https://builds.apache.org/job/HBase-0.94-security/409/ ) HBASE-10505 Import.filterKv does not call Filter.filterRowKey. (larsh: rev 1567522) /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/mapreduce/Import.java
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Committed to 0.94 and 0.96.

        Show
        lhofhansl Lars Hofhansl added a comment - Committed to 0.94 and 0.96.
        Hide
        stack stack added a comment -

        +1 for 0.96

        Show
        stack stack added a comment - +1 for 0.96
        Hide
        lhofhansl Lars Hofhansl added a comment -

        I'd like to address the issues first.
        It's not much work to add a non-matching row/kvs to TestImportExport.testWithFilter, but that would need to be in branches.

        I'll file a subtask for that.

        Show
        lhofhansl Lars Hofhansl added a comment - I'd like to address the issues first. It's not much work to add a non-matching row/kvs to TestImportExport.testWithFilter, but that would need to be in branches. I'll file a subtask for that.
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        Lars Hofhansl
        Patch looks good to me. Can we try adding a filter testcase in testimportExport that validates this behaviour. The current test as you said my be just passing. Even sometime back I found that the testcases in TestImport were just passing without actually getting any KVs. So add a testcase for this behaviour. If it takes time, then we can address in a seperate JIRA.
        +1 on patch.

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - Lars Hofhansl Patch looks good to me. Can we try adding a filter testcase in testimportExport that validates this behaviour. The current test as you said my be just passing. Even sometime back I found that the testcases in TestImport were just passing without actually getting any KVs. So add a testcase for this behaviour. If it takes time, then we can address in a seperate JIRA. +1 on patch.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Ted Yu, stack, you good with the patch? Including 0.96? (Without it filtering in Import is broken)

        Show
        lhofhansl Lars Hofhansl added a comment - Ted Yu , stack , you good with the patch? Including 0.96? (Without it filtering in Import is broken)
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Vasu Mariyala, Jesse Yates, not sure if that any impact on our stuff.

        Show
        lhofhansl Lars Hofhansl added a comment - Vasu Mariyala , Jesse Yates , not sure if that any impact on our stuff.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        0.98 is fine too.

        Show
        lhofhansl Lars Hofhansl added a comment - 0.98 is fine too.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Patches for 0.94 and 0.96 to make it like trunk.

        Show
        lhofhansl Lars Hofhansl added a comment - Patches for 0.94 and 0.96 to make it like trunk.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Heh, I hadn't sync'ed trunk in a but, was just fixed recently. I'll fix it the same way in 0.94 and 0.96 (so it's all the same).

        Show
        lhofhansl Lars Hofhansl added a comment - Heh, I hadn't sync'ed trunk in a but, was just fixed recently. I'll fix it the same way in 0.94 and 0.96 (so it's all the same).
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        In trunk, we have:

              if (filter == null || !filter.filterRowKey(key.get(), key.getOffset(), key.getLength())) {
                for (Cell kv : result.rawCells()) {
        

        So this problem doesn't exist in trunk.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - In trunk, we have: if (filter == null || !filter.filterRowKey(key.get(), key.getOffset(), key.getLength())) { for (Cell kv : result.rawCells()) { So this problem doesn't exist in trunk.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Thanks for fixing this.

        +   * @param row on which to apply the filter
        +   * @return true if the key should not be written, false otherwise
        

        Please add javadoc for other parameters.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Thanks for fixing this. + * @param row on which to apply the filter + * @ return true if the key should not be written, false otherwise Please add javadoc for other parameters.
        Hide
        lhofhansl Lars Hofhansl added a comment -

        Here's a 0.94 patch.
        Actually TestImportExport.testWithFilter only passes by pure accident, because PrefixFilter.filterKeyValue never filtered anything.

        Show
        lhofhansl Lars Hofhansl added a comment - Here's a 0.94 patch. Actually TestImportExport.testWithFilter only passes by pure accident, because PrefixFilter.filterKeyValue never filtered anything.

          People

          • Assignee:
            lhofhansl Lars Hofhansl
            Reporter:
            lhofhansl Lars Hofhansl
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development