HBase
  1. HBase
  2. HBASE-10885 Support visibility expressions on Deletes
  3. HBASE-11054

Create new hook in StoreScanner to help user creating his own delete tracker

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.99.0, 0.98.2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Hide
      Creates a new hook in the RegionObserver Coprocessor,
      {code}
      RegionObserver postInstantiateDeleteTracker(final ObserverContext<RegionCoprocessorEnvironment> ctx, DeleteTracker delTracker)
      {code}
      Introduced in 0.98.2.
      Show
      Creates a new hook in the RegionObserver Coprocessor, {code} RegionObserver postInstantiateDeleteTracker(final ObserverContext<RegionCoprocessorEnvironment> ctx, DeleteTracker delTracker) {code} Introduced in 0.98.2.

      Description

      In case of Visibility labels, to support deletes we need to consider the visibility expression per version inorder to identify the matching visibility expression.
      Tracking the visibility tags along with the delete tracker would make things easier rather than handling during scans/compactions.

      1. HBASE-11054.patch
        11 kB
        ramkrishna.s.vasudevan
      2. HBASE-11054_1.patch
        11 kB
        ramkrishna.s.vasudevan

        Activity

        ramkrishna.s.vasudevan created issue -
        ramkrishna.s.vasudevan made changes -
        Field Original Value New Value
        Fix Version/s 0.99.0 [ 12325675 ]
        Fix Version/s 0.98.2 [ 12326505 ]
        ramkrishna.s.vasudevan made changes -
        Assignee ramkrishna.s.vasudevan [ ram_krish ]
        Hide
        Jean-Marc Spaggiari added a comment -

        Hi Ram,

        Is this similar to HBASE-10115 ?

        JM

        Show
        Jean-Marc Spaggiari added a comment - Hi Ram, Is this similar to HBASE-10115 ? JM
        Hide
        ramkrishna.s.vasudevan added a comment - - edited

        Yes Jean-Marc Spaggiari similar to that. You have patch? I have a patch with me.

        Show
        ramkrishna.s.vasudevan added a comment - - edited Yes Jean-Marc Spaggiari similar to that. You have patch? I have a patch with me.
        Hide
        Jean-Marc Spaggiari added a comment -

        No, I don't have. Created the JIRA to keep track of the need, but did not got a chance to work on that yet.

        Show
        Jean-Marc Spaggiari added a comment - No, I don't have. Created the JIRA to keep track of the need, but did not got a chance to work on that yet.
        Hide
        ramkrishna.s.vasudevan added a comment -

        Patch that adds a new hook when the DeleteTracker is created in the SQM. CPs can implement this to have their own DeleteTrackers.

        Show
        ramkrishna.s.vasudevan added a comment - Patch that adds a new hook when the DeleteTracker is created in the SQM. CPs can implement this to have their own DeleteTrackers.
        ramkrishna.s.vasudevan made changes -
        Attachment HBASE-11054.patch [ 12641510 ]
        ramkrishna.s.vasudevan made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Ted Yu added a comment -

        For RegionObserver.java , three parameters are given in javadoc of preInstantiateDeleteTracker(). But I only see one actual parameter.

        +          deleteTracker = ((RegionObserver) env.getInstance()).preInstantiateDeleteTracker(ctx);
        

        Should deleteTracker from previous invocation be passed to the next invocation ?

        Show
        Ted Yu added a comment - For RegionObserver.java , three parameters are given in javadoc of preInstantiateDeleteTracker(). But I only see one actual parameter. + deleteTracker = ((RegionObserver) env.getInstance()).preInstantiateDeleteTracker(ctx); Should deleteTracker from previous invocation be passed to the next invocation ?
        Hide
        ramkrishna.s.vasudevan added a comment -

        Oh!! I will change it. It happened when i tried removing this patch from a bigger one.

        Should deleteTracker from previous invocation be passed to the next invocation ?

        Ok.. Could do that?
        Let's see whether adding a new hook is fine.

        Show
        ramkrishna.s.vasudevan added a comment - Oh!! I will change it. It happened when i tried removing this patch from a bigger one. Should deleteTracker from previous invocation be passed to the next invocation ? Ok.. Could do that? Let's see whether adding a new hook is fine.
        Hide
        Ted Yu added a comment -

        Can you fill up description with use case ?

        Show
        Ted Yu added a comment - Can you fill up description with use case ?
        ramkrishna.s.vasudevan made changes -
        Description In case of Visibility labels, to support deletes we need to consider the visibility expression per version inorder to identify the matching visibility expression.
        Tracking the visibility tags along with the delete tracker would make things easier rather than handling during scans/compactions.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12641510/HBASE-11054.patch
        against trunk revision .
        ATTACHMENT ID: 12641510

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        -1 javadoc. The javadoc tool appears to have generated 2 warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 lineLengths. The patch does not introduce lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12641510/HBASE-11054.patch against trunk revision . ATTACHMENT ID: 12641510 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. -1 javadoc . The javadoc tool appears to have generated 2 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9373//console This message is automatically generated.
        Hide
        Lars Hofhansl added a comment -

        Should we step back and rethink this code? Can this all be expressed via filters?

        Show
        Lars Hofhansl added a comment - Should we step back and rethink this code? Can this all be expressed via filters?
        Hide
        ramkrishna.s.vasudevan added a comment -

        >>Should we step back and rethink this code? Can this all be expressed via filters?
        You mean the DeleteTrackers itself? Or this patch you mean?
        By the time we call filterKV() in filters the cells masked by deletes are removed already.

        Show
        ramkrishna.s.vasudevan added a comment - >>Should we step back and rethink this code? Can this all be expressed via filters? You mean the DeleteTrackers itself? Or this patch you mean? By the time we call filterKV() in filters the cells masked by deletes are removed already.
        Hide
        Lars Hofhansl added a comment -

        By the time we call filterKV() in filters the cells masked by deletes are removed already.

        I meant generallt (not specifically the patch). We could be radical and say: There are only filters, and all logic is expressed in series of filters. No more deleteTrackers or columnTrackers.

        Show
        Lars Hofhansl added a comment - By the time we call filterKV() in filters the cells masked by deletes are removed already. I meant generallt (not specifically the patch). We could be radical and say: There are only filters, and all logic is expressed in series of filters. No more deleteTrackers or columnTrackers.
        Hide
        Lars Hofhansl added a comment -

        As for this patch. Is a coprocessor the right approach? Or should the class be pluggable? Could make it pluggable at the Store level.
        Do you need find grained programmatic control?

        Show
        Lars Hofhansl added a comment - As for this patch. Is a coprocessor the right approach? Or should the class be pluggable? Could make it pluggable at the Store level. Do you need find grained programmatic control?
        Hide
        ramkrishna.s.vasudevan added a comment -

        There are only filters, and all logic is expressed in series of filters. No more deleteTrackers or columnTrackers.

        +1. True
        Ideally a filterKV() should be able to do this including tracking a delete KV and its versioning.

        Show
        ramkrishna.s.vasudevan added a comment - There are only filters, and all logic is expressed in series of filters. No more deleteTrackers or columnTrackers. +1. True Ideally a filterKV() should be able to do this including tracking a delete KV and its versioning.
        Hide
        ramkrishna.s.vasudevan added a comment -

        Andrew Purtell,larsh
        Can we do the new type of thing in the trunk? But for 0.98.x it may be too big a change right? Do you think the introduction of the new patch would make sense?
        If this is not acceptable then deletion of the visibility labels would become much complex because we will not be able to handle visibility like versions in the StoreScanner or RegionSCanner layer.
        I would love to work on a way where we get rid of trackers and use filters in a seperate JIRA.

        Show
        ramkrishna.s.vasudevan added a comment - Andrew Purtell , larsh Can we do the new type of thing in the trunk? But for 0.98.x it may be too big a change right? Do you think the introduction of the new patch would make sense? If this is not acceptable then deletion of the visibility labels would become much complex because we will not be able to handle visibility like versions in the StoreScanner or RegionSCanner layer. I would love to work on a way where we get rid of trackers and use filters in a seperate JIRA.
        Hide
        Andrew Purtell added a comment -

        As for this patch. Is a coprocessor the right approach? Or should the class be pluggable?

        The component that would use this functionality would be the VisibilityController, which is a coprocessor, operating potentially on some regions but not others.

        There are only filters, and all logic is expressed in series of filters. No more deleteTrackers or columnTrackers.

        That could be fine on trunk but can't happen in 0.98, where we have HBASE-10885 as an important objective.

        Show
        Andrew Purtell added a comment - As for this patch. Is a coprocessor the right approach? Or should the class be pluggable? The component that would use this functionality would be the VisibilityController, which is a coprocessor, operating potentially on some regions but not others. There are only filters, and all logic is expressed in series of filters. No more deleteTrackers or columnTrackers. That could be fine on trunk but can't happen in 0.98, where we have HBASE-10885 as an important objective.
        Hide
        Andrew Purtell added a comment -

        On the design of this particular hook.... Usually with hooks we take in as parameter an existing object and return the same type, which could be the input object with possibly modified state, or a new object allocated within the hook using the input object as a template. The hook here can only blindly create a DeleteTracker and that's not as flexible as if instead there is an option to take the tracker allocated by core and mix in state.

        Show
        Andrew Purtell added a comment - On the design of this particular hook.... Usually with hooks we take in as parameter an existing object and return the same type, which could be the input object with possibly modified state, or a new object allocated within the hook using the input object as a template. The hook here can only blindly create a DeleteTracker and that's not as flexible as if instead there is an option to take the tracker allocated by core and mix in state.
        Hide
        Lars Hofhansl added a comment -

        Thanks for explaining Andrew Purtell. So a coprocessor is the right approach.

        Show
        Lars Hofhansl added a comment - Thanks for explaining Andrew Purtell . So a coprocessor is the right approach.
        Hide
        ramkrishna.s.vasudevan added a comment -

        The hook here can only blindly create a DeleteTracker and that's not as flexible as if instead there is an option to take the tracker allocated by core and mix in state

        I will update the patch. I would pass the core DeleteTracker to this hook and if the CP creates a new deleteTracker would return that else would return the core tracker only.
        Thanks Lars for giving a go ahead with the new hook approach.

        Show
        ramkrishna.s.vasudevan added a comment - The hook here can only blindly create a DeleteTracker and that's not as flexible as if instead there is an option to take the tracker allocated by core and mix in state I will update the patch. I would pass the core DeleteTracker to this hook and if the CP creates a new deleteTracker would return that else would return the core tracker only. Thanks Lars for giving a go ahead with the new hook approach.
        ramkrishna.s.vasudevan made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        ramkrishna.s.vasudevan added a comment - - edited

        Updated patch. Renames the preHook to postHook as now this hook is called after the ScanDeleteTracker is created and the same is passed as param to the hook.
        If no new DeleteTracker is created in the hook the original deletetracker is returned back.

        {Edit}

        hook to deleteTracker.

        Show
        ramkrishna.s.vasudevan added a comment - - edited Updated patch. Renames the preHook to postHook as now this hook is called after the ScanDeleteTracker is created and the same is passed as param to the hook. If no new DeleteTracker is created in the hook the original deletetracker is returned back. {Edit} hook to deleteTracker.
        ramkrishna.s.vasudevan made changes -
        Attachment HBASE-11054_1.patch [ 12641881 ]
        ramkrishna.s.vasudevan made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12641881/HBASE-11054_1.patch
        against trunk revision .
        ATTACHMENT ID: 12641881

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 lineLengths. The patch does not introduce lines longer than 100

        +1 site. The mvn site goal succeeds with this patch.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDistributedLogReplay

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12641881/HBASE-11054_1.patch against trunk revision . ATTACHMENT ID: 12641881 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.security.visibility.TestVisibilityLabelsWithDistributedLogReplay Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9399//console This message is automatically generated.
        Hide
        Andrew Purtell added a comment -

        Also agree that removing trackers and replacing with filters is a good follow on goal Lars Hofhansl and ramkrishna.s.vasudevan

        Show
        Andrew Purtell added a comment - Also agree that removing trackers and replacing with filters is a good follow on goal Lars Hofhansl and ramkrishna.s.vasudevan
        Hide
        Andrew Purtell added a comment -

        +1 on latest patch

        Show
        Andrew Purtell added a comment - +1 on latest patch
        Hide
        ramkrishna.s.vasudevan added a comment -

        Committed to trunk and 0.98. Thanks for the review @apurtell and larsh.
        Created https://issues.apache.org/jira/browse/HBASE-11084 for replacing trackers with filters.

        Show
        ramkrishna.s.vasudevan added a comment - Committed to trunk and 0.98. Thanks for the review @apurtell and larsh . Created https://issues.apache.org/jira/browse/HBASE-11084 for replacing trackers with filters.
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-TRUNK #5120 (See https://builds.apache.org/job/HBase-TRUNK/5120/)
        HBASE-11054-Create new hook in StoreScanner to help user creating his own delete tracker (Ram) (ramkrishna: rev 1590220)

        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
        • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #5120 (See https://builds.apache.org/job/HBase-TRUNK/5120/ ) HBASE-11054 -Create new hook in StoreScanner to help user creating his own delete tracker (Ram) (ramkrishna: rev 1590220) /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-0.98 #297 (See https://builds.apache.org/job/HBase-0.98/297/)
        HBASE-11054-Create new hook in StoreScanner to help user creating his own delete tracker (Ram) (ramkrishna: rev 1590221)

        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java
        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
        • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-0.98 #297 (See https://builds.apache.org/job/HBase-0.98/297/ ) HBASE-11054 -Create new hook in StoreScanner to help user creating his own delete tracker (Ram) (ramkrishna: rev 1590221) /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #282 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/282/)
        HBASE-11054-Create new hook in StoreScanner to help user creating his own delete tracker (Ram) (ramkrishna: rev 1590221)

        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java
        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
        • /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
        • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
        Show
        Hudson added a comment - FAILURE: Integrated in HBase-0.98-on-Hadoop-1.1 #282 (See https://builds.apache.org/job/HBase-0.98-on-Hadoop-1.1/282/ ) HBASE-11054 -Create new hook in StoreScanner to help user creating his own delete tracker (Ram) (ramkrishna: rev 1590221) /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java /hbase/branches/0.98/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestQueryMatcher.java
        ramkrishna.s.vasudevan made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Resolution Fixed [ 1 ]
        Hide
        Nicolas Liochon added a comment -

        This changes the interface of the RegionObserver in 0.98.2 vs 0.98.1, so it impacts an application that implements it.
        Do we have a defined policy for this?
        It's not that much of a an issue, especially as we're still at the beginning of the 0.98, but it would worth mentioning it in the release notes imho...

        Show
        Nicolas Liochon added a comment - This changes the interface of the RegionObserver in 0.98.2 vs 0.98.1, so it impacts an application that implements it. Do we have a defined policy for this? It's not that much of a an issue, especially as we're still at the beginning of the 0.98, but it would worth mentioning it in the release notes imho...
        Hide
        Andrew Purtell added a comment -

        Do we have a defined policy for this?

        None that I am aware of beyond "let's stop doing this for 1.0 and beyond", please see related issue HBASE-11125

        Show
        Andrew Purtell added a comment - Do we have a defined policy for this? None that I am aware of beyond "let's stop doing this for 1.0 and beyond", please see related issue HBASE-11125
        Hide
        ramkrishna.s.vasudevan added a comment -

        but it would worth mentioning it in the release notes imho...

        Okie. Just seeing this ping in the JIRA.

        Show
        ramkrishna.s.vasudevan added a comment - but it would worth mentioning it in the release notes imho... Okie. Just seeing this ping in the JIRA.
        ramkrishna.s.vasudevan made changes -
        Hadoop Flags Reviewed [ 10343 ] Incompatible change,Reviewed [ 10342, 10343 ]
        Release Note Creates a new hook in the RegionObserver Coprocessor,
        {code}
        RegionObserver postInstantiateDeleteTracker(final ObserverContext<RegionCoprocessorEnvironment> ctx, DeleteTracker delTracker)
        {code}
        Introduced in 0.98.2.
        Hide
        Nicolas Liochon added a comment -

        None that I am aware of beyond "let's stop doing this for 1.0 and beyond", please see related issue HBASE-11125

        Ok, that's fine for me.

        Okie. Just seeing this ping in the JIRA.

        Thanks, Ram!

        Show
        Nicolas Liochon added a comment - None that I am aware of beyond "let's stop doing this for 1.0 and beyond", please see related issue HBASE-11125 Ok, that's fine for me. Okie. Just seeing this ping in the JIRA. Thanks, Ram!
        Hide
        stack added a comment -

        We breaking anyone? Phoenix? I'd say policy should be no changes to method apis across minor versions?

        Show
        stack added a comment - We breaking anyone? Phoenix? I'd say policy should be no changes to method apis across minor versions?
        Hide
        Anoop Sam John added a comment -

        We added a new cp method in RegionObserver. If a user having a cp impl, directly implementing RegionObserver (not extending BaseRegionObserver) the addition of new method will break compatibility.

        Show
        Anoop Sam John added a comment - We added a new cp method in RegionObserver. If a user having a cp impl, directly implementing RegionObserver (not extending BaseRegionObserver) the addition of new method will break compatibility.
        Hide
        Andrew Purtell added a comment -

        This is a new hook, we are not breaking anyone

        I'd say policy should be no changes to method apis across minor versions

        That would prevent any changes at all to coprocessor APIs in 0.98, we'd have to leave the branch behind and move to trunk only.

        Show
        Andrew Purtell added a comment - This is a new hook, we are not breaking anyone I'd say policy should be no changes to method apis across minor versions That would prevent any changes at all to coprocessor APIs in 0.98, we'd have to leave the branch behind and move to trunk only.
        Hide
        ramkrishna.s.vasudevan added a comment -

        Seeing these comments late.
        Generally it is advisable to use BaseRegionObserver only but if RegionObserver is used then there will be a compilation issue when using the CP application with 0.98.2. If we cannot add new hooks in 0.98 then we will not be able to add any new improvements that could be done using hooks.

        Show
        ramkrishna.s.vasudevan added a comment - Seeing these comments late. Generally it is advisable to use BaseRegionObserver only but if RegionObserver is used then there will be a compilation issue when using the CP application with 0.98.2. If we cannot add new hooks in 0.98 then we will not be able to add any new improvements that could be done using hooks.
        Hide
        Nicolas Liochon added a comment -

        I wonder is there is a good reason to extend the RegionObserver directly? In this case it could become private?

        Now that I think about it again, I think that we should always add a @since annotation as well (not for this change, but for the next ones). This way the developer knows that he is using an API not available in a previous dot release (we can easily imagine that multiple distros may have different underlying hbase dot version).

        Show
        Nicolas Liochon added a comment - I wonder is there is a good reason to extend the RegionObserver directly? In this case it could become private? Now that I think about it again, I think that we should always add a @since annotation as well (not for this change, but for the next ones). This way the developer knows that he is using an API not available in a previous dot release (we can easily imagine that multiple distros may have different underlying hbase dot version).
        Hide
        Enis Soztutar added a comment -

        Closing this issue after 0.99.0 release.

        Show
        Enis Soztutar added a comment - Closing this issue after 0.99.0 release.
        Enis Soztutar made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        1d 14h 16m 1 ramkrishna.s.vasudevan 25/Apr/14 08:15
        Open Open Patch Available Patch Available
        5h 22m 2 ramkrishna.s.vasudevan 25/Apr/14 08:16
        Patch Available Patch Available Resolved Resolved
        2d 23h 9m 1 ramkrishna.s.vasudevan 28/Apr/14 07:26
        Resolved Resolved Closed Closed
        299d 17h 6m 1 Enis Soztutar 21/Feb/15 23:32

          People

          • Assignee:
            ramkrishna.s.vasudevan
            Reporter:
            ramkrishna.s.vasudevan
          • Votes:
            0 Vote for this issue
            Watchers:
            11 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development