HBase
  1. HBase
  2. HBASE-10505

Import.filterKv does not call Filter.filterRowKey

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical 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
        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
        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.
        Hide
        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
        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
        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
        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
        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
        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
        Lars Hofhansl added a comment -

        Patches for 0.94 and 0.96 to make it like trunk.

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

        0.98 is fine too.

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

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

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

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

        Show
        Lars Hofhansl added a comment - Ted Yu , stack , you good with the patch? Including 0.96? (Without it filtering in Import is broken)
        Hide
        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
        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
        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
        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
        stack added a comment -

        +1 for 0.96

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

        Committed to 0.94 and 0.96.

        Show
        Lars Hofhansl added a comment - Committed to 0.94 and 0.96.
        Hide
        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 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
        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 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 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 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 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 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 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 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 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 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
        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 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
        Andrew Purtell added a comment -

        Why not branches > 0.96?

        Show
        Andrew Purtell added a comment - Why not branches > 0.96?
        Hide
        Vasu Mariyala added a comment -

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

        Show
        Vasu Mariyala added a comment - For branches > 0.96, the issue is fixed with HBASE-10416
        Hide
        Lars Hofhansl added a comment -

        Yeah, this is fixed in 0.98 and later.

        Show
        Lars Hofhansl added a comment - Yeah, this is fixed in 0.98 and later.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development