Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-11871 Avoid usage of KeyValueUtil#ensureKeyValue
  3. HBASE-12068

[Branch-1] Avoid need to always do KeyValueUtil#ensureKeyValue for Filter transformCell

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.99.0
    • Fix Version/s: 0.99.1
    • Component/s: Filters
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      When custom Filters are used, make sure to override Cell based methods and not keyValue based methods. ie.
      transformCell(Cell) instead of transform(KeyValue)
      getNextCellHint(Cell) instead of getNextKeyHint(KeyValue)
      Else there can be possible performance hit of a conversion of a non KV Cell to a KV which required key and value deep copy.
      Show
      When custom Filters are used, make sure to override Cell based methods and not keyValue based methods. ie. transformCell(Cell) instead of transform(KeyValue) getNextCellHint(Cell) instead of getNextKeyHint(KeyValue) Else there can be possible performance hit of a conversion of a non KV Cell to a KV which required key and value deep copy.

      Description

      During read with Filters added to Scan/Get, the core code calls transformCell(Cell) on the Filter. Most of the filters do not implement transform API so the method from FilterBase will get executed

        @Override
        public Cell transformCell(Cell v) throws IOException {
          // Old filters based off of this class will override KeyValue transform(KeyValue).
          // Thus to maintain compatibility we need to call the old version.
          return transform(KeyValueUtil.ensureKeyValue(v));
        }
      

      Here always it do KeyValueUtil.ensureKeyValue. When a non KV cell comes in, we need recreate KV and do deep copy of key and value!
      We have to stick with this model in branch-1 for BC.
      So as a workaround to avoid possible KV convert, we can implement transformCell(Cell) method in all of our individual Filter classes which just return the incoming cell (So that method from FilterBase wont get executed)

      1. HBASE-12068.patch
        30 kB
        Anoop Sam John

        Issue Links

          Activity

          Hide
          anoop.hbase Anoop Sam John added a comment -

          The patch also avoids KeyValueUtil#ensureKeyValue calls from DependentColumnFilter, SingleColumnValueExcludeFilter and SingleColumnValueFilter. All where simple cases for removal. (These calls not present trunk code)

          Show
          anoop.hbase Anoop Sam John added a comment - The patch also avoids KeyValueUtil#ensureKeyValue calls from DependentColumnFilter, SingleColumnValueExcludeFilter and SingleColumnValueFilter. All where simple cases for removal. (These calls not present trunk code)
          Hide
          enis Enis Soztutar added a comment -

          +1. Thanks Anoop. Should we also ensure Phoenix Filters also override transformCell() as a follow up.

          Show
          enis Enis Soztutar added a comment - +1. Thanks Anoop. Should we also ensure Phoenix Filters also override transformCell() as a follow up.
          Hide
          stack stack added a comment -

          +1

          On commit add comment to each instance where you are doing this pass-through. Otherwise I see folks looking at it and just being stumped why. Add release note. I added note to our 1.0 issue that we need to call this one out in release notes: "If you have written your own filter..."

          Show
          stack stack added a comment - +1 On commit add comment to each instance where you are doing this pass-through. Otherwise I see folks looking at it and just being stumped why. Add release note. I added note to our 1.0 issue that we need to call this one out in release notes: "If you have written your own filter..."
          Hide
          enis Enis Soztutar added a comment -

          release notes: "If you have written your own filter..."

          +1. That would be good mentioning that you better implement the new method yourself for better performance.

          Show
          enis Enis Soztutar added a comment - release notes: "If you have written your own filter..." +1. That would be good mentioning that you better implement the new method yourself for better performance.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          On commit add comment to each instance where you are doing this pass-through. Otherwise I see folks looking at it and just being stumped why. Add release note. I added note to our 1.0 issue that we need to call this one out in release notes: "If you have written your own filter..."

          Sure.
          Thanks for the review Stack & Enis.
          Yes PHOENIX-1282 is the issue in Phoenix side for the followup there. In Phoenix master will have to do. 4.0 is based on 0.98 and there all are KeyValues any way.

          Show
          anoop.hbase Anoop Sam John added a comment - On commit add comment to each instance where you are doing this pass-through. Otherwise I see folks looking at it and just being stumped why. Add release note. I added note to our 1.0 issue that we need to call this one out in release notes: "If you have written your own filter..." Sure. Thanks for the review Stack & Enis. Yes PHOENIX-1282 is the issue in Phoenix side for the followup there. In Phoenix master will have to do. 4.0 is based on 0.98 and there all are KeyValues any way.
          Hide
          anoop.hbase Anoop Sam John added a comment -

          Pushed to branch-1. Thanks for the reviews Stack & Enis.

          Show
          anoop.hbase Anoop Sam John added a comment - Pushed to branch-1. Thanks for the reviews Stack & Enis.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in HBase-1.0 #224 (See https://builds.apache.org/job/HBase-1.0/224/)
          HBASE-12068 [Branch-1] Avoid need to always do KeyValueUtil#ensureKeyValue for Filter transformCell. (anoopsamjohn: rev af35daac777c06639d3eb1985def4846fda84cbb)

          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/RandomRowFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/PageFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/InclusiveStopFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FirstKeyOnlyFilter.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueExcludeFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnRangeFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/PrefixFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnPrefixFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FuzzyRowFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnPaginationFilter.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/filter/FilterAllFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/TimestampsFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterBase.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/MultipleColumnPrefixFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/CompareFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterWrapper.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelFilter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnCountGetFilter.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in HBase-1.0 #224 (See https://builds.apache.org/job/HBase-1.0/224/ ) HBASE-12068 [Branch-1] Avoid need to always do KeyValueUtil#ensureKeyValue for Filter transformCell. (anoopsamjohn: rev af35daac777c06639d3eb1985def4846fda84cbb) hbase-client/src/main/java/org/apache/hadoop/hbase/filter/RandomRowFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/PageFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/InclusiveStopFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FirstKeyOnlyFilter.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueExcludeFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnRangeFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/DependentColumnFilter.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessControlFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/SingleColumnValueFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/PrefixFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnPrefixFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FuzzyRowFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnPaginationFilter.java hbase-server/src/test/java/org/apache/hadoop/hbase/filter/FilterAllFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/TimestampsFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterBase.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/MultipleColumnPrefixFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/CompareFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterWrapper.java hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityLabelFilter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/Filter.java hbase-client/src/main/java/org/apache/hadoop/hbase/filter/ColumnCountGetFilter.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in HBase-TRUNK #5714 (See https://builds.apache.org/job/HBase-TRUNK/5714/)
          Add note to upgrade section on HBASE-12068; i.e. things to do if you have custom filters (stack: rev 3a9cf5b2cdc3c3d24a085b3a4d4c289dcce09766)

          • src/main/docbkx/upgrading.xml
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #5714 (See https://builds.apache.org/job/HBase-TRUNK/5714/ ) Add note to upgrade section on HBASE-12068 ; i.e. things to do if you have custom filters (stack: rev 3a9cf5b2cdc3c3d24a085b3a4d4c289dcce09766) src/main/docbkx/upgrading.xml
          Hide
          enis Enis Soztutar added a comment -

          Closing this issue after 0.99.1 release.

          Show
          enis Enis Soztutar added a comment - Closing this issue after 0.99.1 release.

            People

            • Assignee:
              anoop.hbase Anoop Sam John
              Reporter:
              anoop.hbase Anoop Sam John
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development