[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[
https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904366#comment-16904366
]
Hudson commented on HBASE-22623:
Results for branch branch-2
[build #2158 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2158/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2158//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2158//JDK8_Nightly_Build_Report_(Hadoop2)/]
(/) {color:green}+1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/2158//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> Add RegionObserver coprocessor hook for preWALAppend
>
>
> Key: HBASE-22623
> URL: https://issues.apache.org/jira/browse/HBASE-22623
> Project: HBase
> Issue Type: New Feature
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Fix For: 3.0.0, 1.5.0, 2.3.0
>
>
> While many coprocessor hooks expose the WALEdit to implementing coprocs,
> there aren't any that expose the WALKey before it's created and added to the
> WALEntry.
> It's sometimes useful for coprocessors to be able to edit the WALKey, for
> example to add extended attributes using the fields to be added in
> HBASE-22622.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[
https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904359#comment-16904359
]
Hudson commented on HBASE-22623:
Results for branch branch-1
[build #1000 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-1/1000/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(x) {color:red}-1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-1/1000//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk7 checks{color}
-- For more information [see jdk7
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-1/1000//JDK7_Nightly_Build_Report/]
(/) {color:green}+1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-1/1000//JDK8_Nightly_Build_Report_(Hadoop2)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
> Add RegionObserver coprocessor hook for preWALAppend
>
>
> Key: HBASE-22623
> URL: https://issues.apache.org/jira/browse/HBASE-22623
> Project: HBase
> Issue Type: New Feature
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Fix For: 3.0.0, 1.5.0, 2.3.0
>
>
> While many coprocessor hooks expose the WALEdit to implementing coprocs,
> there aren't any that expose the WALKey before it's created and added to the
> WALEntry.
> It's sometimes useful for coprocessors to be able to edit the WALKey, for
> example to add extended attributes using the fields to be added in
> HBASE-22622.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[
https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904345#comment-16904345
]
Hudson commented on HBASE-22623:
Results for branch master
[build #1324 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/master/1324/]: (x)
*{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/master/1324//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/master/1324//JDK8_Nightly_Build_Report_(Hadoop2)/]
(/) {color:green}+1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/master/1324//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> Add RegionObserver coprocessor hook for preWALAppend
>
>
> Key: HBASE-22623
> URL: https://issues.apache.org/jira/browse/HBASE-22623
> Project: HBase
> Issue Type: New Feature
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Fix For: 3.0.0, 1.5.0, 2.3.0
>
>
> While many coprocessor hooks expose the WALEdit to implementing coprocs,
> there aren't any that expose the WALKey before it's created and added to the
> WALEntry.
> It's sometimes useful for coprocessors to be able to edit the WALKey, for
> example to add extended attributes using the fields to be added in
> HBASE-22622.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904150#comment-16904150 ] Andrew Purtell commented on HBASE-22623: Thanks Stack, agree, follow up issues are most welcome > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904149#comment-16904149 ] Andrew Purtell commented on HBASE-22623: Thanks Geoffrey. I will have time to commit this later today. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904119#comment-16904119 ] stack commented on HBASE-22623: --- bq. "...but what I'm trying to understand is why. " Because we were afraid to expose this fundamental for others to manipulate. WALEdit shows up in a bunch of contexts. There is the original write to the WAL; there is the WAL Replay; and there is replication-time. Any mis-accounting of Cells or sizes would mess us up. Debugging a cluster with missing or mis-accounted edits is a time sink. We were being conservative. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904052#comment-16904052 ] Geoffrey Jacoby commented on HBASE-22623: - Branch-1 backport PR is up at 470. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904022#comment-16904022 ] Geoffrey Jacoby commented on HBASE-22623: - [~apurtell] branch-1 backport coming in the next hour or two. Just doing final polish. It wasn't too bad, but still non-trivial because each class of operation has its own WAL append code in branch-1. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904017#comment-16904017 ] Andrew Purtell commented on HBASE-22623: [~gjacoby] Did you say you have a branch-1 backport? Otherwise it looks straightforward enough, I could do it. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904015#comment-16904015 ] Andrew Purtell commented on HBASE-22623: Ok, good for commit then. Will commit shortly. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16903256#comment-16903256 ] Andrew Purtell commented on HBASE-22623: Waiting 24 hours for time zone turn around. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16903253#comment-16903253 ] Andrew Purtell commented on HBASE-22623: I sent an email with the subject "Coprocessors, clean ups, compatibility, deprecations, Phoenix... it's a bit of a mess" to dev@ > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16903241#comment-16903241 ] Andrew Purtell commented on HBASE-22623: Just to be clear because I used the word "hostile" which can have negative connotations - I don't mean it in a negative way. An opinion that we should have more and more coprocessor interfaces to address new use cases is valid. An opinion that coprocessors are too invasive and should be 'cleaned up' is also valid. An opinion that the compatibility headaches of coprocessor interfaces are annoying is valid. An opinion that Phoenix can be considered as a valid use case when considering interface changes is valid. An opinion that only HBase level concerns should motivate API changes is valid. These opinions are strawmen. I think they approach actual positions in the community but I do not imply any specific person has one of them. These strawmen are at least partially contradictory. It is going to be an ongoing process to sort them out into something that makes sense and can get consensus. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16903233#comment-16903233 ] Andrew Purtell commented on HBASE-22623: I am merging the current PR and [~gjacoby] has indicated a backport patch to branch-1 is forthcoming, which I will wait for. I'll pick to the branch-2, that looks straightforward. Let me repeat some comments I left on the PR here because it is a PMC level issue that should be addressed I think. Getting visibility: The new method is not going to be deprecated out of the bat. (I agree with [~gjacoby] this is nonsensical and the conversation here is getting more confusing because of an underlying misalignment IMHO). We are not monolithic in our approach (and hostility) to coprocessor interfaces as a community. Imposing that disagreement on contributors is not appropriate. I am going to merge this as is and we can follow up on what should or should not be deprecated as a larger conversation on the future of coprocessors. The community has some big disagreements in approach. I also agree the "policy", such as it is, is contradictory and confusing. We need to attack the bigger picture on dev@ in a discussion about the future of coprocessors and our tolerance (or not) to the requests of the Phoenix project. The opinions are not monolithic. There are some supporters, there are some hostile positions, both are valid in my view, we need to sort out the disagreement. This issue isn't the right scope for that. The contradictory positions are evident in the tug and pull of the suggestions to the contributor. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16903227#comment-16903227 ] Andrew Purtell commented on HBASE-22623: I don't think the new method should be deprecated. It has an intended user. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902674#comment-16902674 ] Duo Zhang commented on HBASE-22623: --- I mean the newly added coprocessor method. Let me comment on github. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902673#comment-16902673 ] Geoffrey Jacoby commented on HBASE-22623: - [~Apache9] - I apologize for being rude in my last reply. WALObserver vs RegionObserver's an easy mistake to make. What method would you like to be marked as deprecated? preWALWrite? Looks like it already is. As for WALEdit, the original comments warning developers not to touch it remain in the current version of the patch, as do the IA.Private attributes on the mutator methods like add. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902621#comment-16902621 ] Duo Zhang commented on HBASE-22623: --- Oh, I misread the patch. I thought it is in WALObserver. Sorry. Then please mark it as deprecated, and mention that WALEdit is IA.Private so do not touch it if possible. Thanks. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902619#comment-16902619 ] Andrew Purtell commented on HBASE-22623: I think we’re just having a bit of a miscommunication. I read Duo’s comment as a suggestion to move the current WALObserver method over to RegionObserver. However I believe that the wrong approach because it breaks both interfaces. At least with the current patch we are binary compatible and source incompatible only in RegionObserver. I plan to commit it tomorrow > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902615#comment-16902615 ] Geoffrey Jacoby commented on HBASE-22623: - [~Apache9] - the new method is in RegionObserver, and always has been. Did you read the patch you rejected? > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902616#comment-16902616 ] Andrew Purtell commented on HBASE-22623: This patch does place the method in RegionObserver. Without making any incompatible changes elsewhere. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[
https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902612#comment-16902612
]
Duo Zhang commented on HBASE-22623:
---
{quote}
Oh sorry. Please pardon the brevity. Not at home right now. The new
RegionObserver hook is needed because the thread context of the WALobserver is
not the right one, we need the hook to have the RPC thread context. The latest
patch lgtm
{quote}
Then I think this method should be placed in RegionObserver, not WALObserver...
> Add RegionObserver coprocessor hook for preWALAppend
>
>
> Key: HBASE-22623
> URL: https://issues.apache.org/jira/browse/HBASE-22623
> Project: HBase
> Issue Type: New Feature
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Fix For: 3.0.0, 1.5.0, 2.3.0
>
>
> While many coprocessor hooks expose the WALEdit to implementing coprocs,
> there aren't any that expose the WALKey before it's created and added to the
> WALEntry.
> It's sometimes useful for coprocessors to be able to edit the WALKey, for
> example to add extended attributes using the fields to be added in
> HBASE-22622.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902611#comment-16902611 ] Andrew Purtell commented on HBASE-22623: I think pre WAL write is in WALObserver but is not called from the right thread context. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902608#comment-16902608 ] Geoffrey Jacoby commented on HBASE-22623: - 1. There does not appear to be a preWALWrite in RegionObserver that I can see in master, and I don't see it called in doWALAppend. If I'm missing something, please let me know -- figuring out the hooks and their ordering isn't well-documented. The coprocessor hook needs to be in RegionObserver along with all the other mutation-related coprocessor hooks, because of shared context. 2. For my particular use case, annotating the WALKey as provided in HBASE-22622 is sufficient. [~apurtell] pointed out during the review that other use cases might have need of the edit, and that it would be trivial to add it now rather than need a breaking change later, which I agree with. (3). In the hypothetical, no-longer-relevant case where you added extra Cells to the WALEdit but not the MemStore, you would presumably have the replication endpoint that consumed them filter them out from being replicated to the remote peer cluster. The are Cell-level filtering APIs in the replication code. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902605#comment-16902605 ] Andrew Purtell commented on HBASE-22623: The thread context is not the RPC thread. Without this we have no means to get the right state in place unless Geoffrey has another idea. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902603#comment-16902603 ] Duo Zhang commented on HBASE-22623: --- Can anyone answer my first question? I've asked several times, what is the difference between the old preWALWrite and the new preWALAppend? Your logic can not be implemented using the old preWALWrite? Why? > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902602#comment-16902602 ] Andrew Purtell commented on HBASE-22623: Oh sorry. Please pardon the brevity. Not at home right now. The new RegionObserver hook is needed because the thread context of the WALobserver is not the right one, we need the hook to have the RPC thread context. The latest patch lgtm > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902598#comment-16902598 ] Andrew Purtell commented on HBASE-22623: It is about the WALkey for earlier described reasons. I have also explained my position about why also including a parameter for WALedit in multiple earlier comments and don’t need to repeat it again. The latest patch seems to strike the right balance by avoiding concerns about WALedit as you rightly point out is not needed now anyway so is a bit theoretical. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902590#comment-16902590 ] Duo Zhang commented on HBASE-22623: --- Two problems: 1. Why not just use the old preWALWrite method? Althrough it is marked as deprecated, we will not remove it unless we find an alternate method which can do almost the same thing. The new preWALAppend is almost the same with preWALWrite, just with a different method name. 2. I thought you guys just want to add some attributes to WALKey, although I still have some concerns but at least, I do not think adding extended atrributes will break critical assumptions. Then why here we changed the topic to amending the WALEdit? What is the usage? IIRC, in another issue, Stack and I suggest that you could use META_FAMILY without adding new attributes to WALKey but you guys refused? Then what's going on here? I'm a bit confused. And what's more, adding cells to WALEdit but no memstore is a no no. After replication, the data will be replicated to peer cluster, with the cells not in memstore but in WALEdit, and also, when replaying, you will see the cells in WALEdit but not in memstore. That means, you have to intercept the put/delete operations, both in the normal write path and the fail recovery path, then why not put all the logic there, instead of implementing a sperated logic at the WAL layer? This does not make sense... > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902517#comment-16902517 ] Geoffrey Jacoby commented on HBASE-22623: - [~apurtell] I have modified the patch to revert the comment and InterfaceAudience changes to WALEdit (so that add is still marked Private) and added a check to HRegion.doWALAppend to not call the new coproc hook when WALEdit.isMetaEdit() is true. I have also included a test to verify the latter. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902242#comment-16902242 ] Andrew Purtell commented on HBASE-22623: Also for clarification I think we can include a WALEdit parameter in the hook even if it is read only. That would be better than nothing, and would not be dumb. Maybe we can just table this discussion because only the WALKey needs to be modified at this time. The rathole about whether or not WALedit is immutable can be addressed later. (Still kind of silly in my opinion, but there have been multiple expressions of concern about this so my opinion may be in the minority.) > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902239#comment-16902239 ] Andrew Purtell commented on HBASE-22623: For the record I am not -1 to the previous version of the patch which did not include WALedit in the new hook. I am however of the opinion that the goal is to intercept WALentries, and they are composed of WALKey and WALEdit, so adding a hook that only has WALKey and not WALEdit would be dumb. We can do that as "compromise". It would be a dumb outcome, though, collectively, we would make a dumb engineering decision. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902234#comment-16902234 ] Andrew Purtell commented on HBASE-22623: I have to agree. I can call System.exit() from a coprocessor. That doesn't mean it is a good idea. A necessary precondition for coprocessor API use is an understanding you are messing with internals. It's not meant for arbitrary user code or user development. We don't need to protect against dumb users. What we can/should protect is our ability to change LP(coproc) interfaces, so if there is a concern there that would certainly be valid. However these classes - WALKey and WALEdit are both already LP(coproc). I think [~Apache9]'s position is not reasonable, but he is entitled to his opinion. I hope he will consider changing it, though. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16902136#comment-16902136 ] Geoffrey Jacoby commented on HBASE-22623: - So, as things stand now, [~apurtell] is -1 to a previous version of the patch that didn't contain WALEdit in the hook, and [~Apache9] is -1 to the current version that does contain it, leaving the null set. So where do we go from here to resolve things? I think [~anoop.hbase]'s last point is critical -- it's theoretically possible to accomplish "adding a Cell to the WALEdit of every mutation", but it's both 1. Incredibly complicated AND 2. Impossible to add to the WALEdit but not the MemStore / HFiles, which would be desirable in cases where you want to annotate metadata for consumption by a replication endpoint but not end-users of the client API. (Please correct me if I'm wrong) 3. Brittle anytime anyone changes any of the affected coprocessor hooks Spreading the code around also violates the single responsibility principle -- it's best to have all the code for a particular purpose in a single particular place. Between having to write over half a dozen coprocessor hooks with very different semantics (batch mutate hooks look different from single mutation hooks, append and increment look different from Put, etc), our hypothetical coproc developer needs to know and understand an enormous variety of concepts, not to mention the ordering of the entire write pipeline. Note how the discussion between Duo and Anoop above is finding it non-trivial, and you guys are among the foremost experts in the world on this. What exactly are the expectations for coproc developers? You seem to be assuming simultaneously that coproc developers need to be heavily constrained because their lack of knowledge is dangerous, but at the same time they are wise to be able to figure out complex workarounds to those constraints. (And to know when those complex workarounds are good (your proposal for WAL cell addition above) vs bad (you dislike the local index implementation).) Just exposing the WALEdit gives a simple, clean API that can be implemented in a few lines of code and isn't any more dangerous than any other existing coproc hook. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901820#comment-16901820 ] Anoop Sam John commented on HBASE-22623: That will be too much of a work for such a need IMHO. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901797#comment-16901797 ] Duo Zhang commented on HBASE-22623: --- In the preIncrement and preAppend, you can return a Result so that the later operations will be bypassed. This means you can do everything in this method, such as convert the operation to a batch of puts/deletes. I think this is the suggested way to intercept increment/append in HBase. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901793#comment-16901793 ] Anoop Sam John commented on HBASE-22623: preAppend/Increment CP APIs takes Append/Increment only. The WALEdit is not yet created at that time. The need in Phoenix for which this jira been raised want to add some extra Cell into WALEdit only. Dont want to persist any extra Cells into Memstore. If they add the Cell in preAppend/Increment that is not possible right? Also we should handle the issue which I mention above regarding the MiniBatchOperationInProgress and setting new WALEdit into it in CP hook. Well that can be fixed if we had a static Util API to create WALEdit new object by taking N cells. But we should fix it. Also at least one method is missing Private annotation if we want to do that for all Cell addition APIs. May be another Jira we can open for that. But this jira made us to see those issues. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[
https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901757#comment-16901757
]
Duo Zhang commented on HBASE-22623:
---
{quote}
Because MiniBatchOperationInProgress and related CP APIs are not available with
Append/Increment kinda Mutations
{quote}
But we have preIncrement and preAppend? Why not use these APIs?
> Add RegionObserver coprocessor hook for preWALAppend
>
>
> Key: HBASE-22623
> URL: https://issues.apache.org/jira/browse/HBASE-22623
> Project: HBase
> Issue Type: New Feature
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Fix For: 3.0.0, 1.5.0, 2.3.0
>
>
> While many coprocessor hooks expose the WALEdit to implementing coprocs,
> there aren't any that expose the WALKey before it's created and added to the
> WALEntry.
> It's sometimes useful for coprocessors to be able to edit the WALKey, for
> example to add extended attributes using the fields to be added in
> HBASE-22622.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901754#comment-16901754 ] Duo Zhang commented on HBASE-22623: --- Anyway I'm -1 on adding a new method which is just a copy of an old deprecated method. Please try to abstract a WALEdit interface and only add what you want into it without exposing all the details, or you can just use the old preWALWriter thing as IIRC, you just want the WALKey attributes? And thanks for taking us to the WALEntryFilter, this is exactly what I said above, the classes which need to be refactored. We even expose WALKeyImpl in the API which is not good. A filter should be read only. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901694#comment-16901694 ] Geoffrey Jacoby commented on HBASE-22623: - [~Apache9] - any coprocessor hook can create data loss if used maliciously or incorrectly, because they allow arbitrary code execution; I gave examples above. But to turn back to the patch... I definitely see your and [~anoop.hbase]'s point about the meta cell addition. Making the coproc authors need to know about low level HBase details like that would make it easy to make mistakes and require boilerplate code in almost every hook implementation. I will incorporate that into the patch. But WALEdit isn't some strange, exotic class that's never been exposed to client developers. WAL.Entry, WALKey, and WALEdit are already part of the WALEntryFilter / ReplicationEndpoint API, which I've worked with wearing both my Phoenix and day-job hats. I can already add a Cell to a Put through a coproc, and I can add a Cell to the WALEdit bound for a _remote_ cluster by a custom replication endpoint. Other coproc hooks after the WAL sync expose the WALEdit too. The fact that I have existing cumbersome ways to accomplish this isn't a reason not to give me a straightforward one, but the opposite. It's most often when developers try to do cumbersome workarounds that we get into trouble. :-) > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901677#comment-16901677 ] Anoop Sam John commented on HBASE-22623: In this discussion one comment on patch come up. The new hook will be called for each and every add ops to WAL. So when the add is doing a META cell addition we should not call the hook. You need to add the check. This is comment on the patch. [~Apache9] Pls see my comment abt the MiniBatchOperationInProgress related CP API around batchMutate() flow. Here the CP is expected to create new WALEdit object and set to MiniBatchOperationInProgress. But how the CP impl can create a WALEdit with cells(). There is no constructor which takes cells. The add() methods are Private marked. There is no static util method or WALEdit builder to do this. Only add(Map> familyMap) is not marked private. But I think that is a miss at that time. If add(Cell) is private then this add(Map) also to be same way. Also getCells() return the same List instance and CP can make use. This is not marked with any and no Javadoc even. All these means there is some thing missing on our end already. The 3rd is the discussion in this Jira specific. I agree to the point that there should be ways to add some extra Cells into the WALEdit along with this API. Because MiniBatchOperationInProgress and related CP APIs are not available with Append/Increment kinda Mutations. Now how to achieve that goal of letting CPs to add extra Cells into same WALEdit is a topic of discussion. So lets address one by one I would say. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901676#comment-16901676 ] Duo Zhang commented on HBASE-22623: --- At least Thread.sleep and System.exit will not mess up the data right? As I said above, if you want to put things into WALEdit, just use higher level CPs to add more puts/deletes into the batch. I know this would be a pain, but I think this is the correct way. We used to expose too many internal things in CPs which made Phoenix went to the wrong way, for example, the strange local secondary index implementation which breaks the HBase assumption after split/merge, and then hack deeply into the StoreFileReader which highly depends on the private implementation of HBase so after upgrading to 2.x everything is broken... And back to WALKey&WALEdit, even though I still has some concerns on HBASE-22622, but at least, WALKey is an interface, and the only write method is addExtendedAttribute, which I think is fine. But for WALEdit, it is an implementation class, it even has setCells, readFromCells method, which will be really dangerous to CP users. Notice that now we are making everything off heap, and we have just fixed a critical bug related to reference counting on cells, and here we let users directly set or remove cells? I do not think this is a good idea... And for the patch here, what is the difference between preWALAppend and the old preWALWrite? > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901668#comment-16901668 ] Geoffrey Jacoby commented on HBASE-22623: - [~Apache9] - that seems like an argument that the new coproc hook shouldn't fire for "meta" WALEdits, just ones associated with user operations, rather than an argument against not exposing mutable WALEdit to coprocs in general. If we were to take that approach, would that just require a check to WALEdit.isMetaEdit(), or can user and system level Cells be interleaved in the same Edit? To your second point, every single coprocessor hook in HBase can be used to "break the logic of HBase", by their very nature, because an evil coproc developer can say "Thread.sleep(Long.MAX_VALUE)", "System.exit(1)", or even start deleting WAL or data directories. An operator who runs a coprocessor is trusting the coproc to the same extent they trust HBase. IMO we should be making it easy to do useful things correctly, and harder to do bad things accidentally, but we're never going to make coprocessors as they exist now "safe". > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901629#comment-16901629 ] Duo Zhang commented on HBASE-22623: --- For example, we will write some marker edits to WAL, and it will be used in many places. If in the CP you delete it or overwrite it with other values, the logic in HBase itself will be broken and cause unexpected behavior. That's what we say 'messed up'. The upper level coprocessors can only effect user related data, for example you can add more puts or deletes into a batch, but it will not effect the internal things. And if you find other things which will break the logic of HBase, please tell me, I will file issues to remove the CP methods. Thanks. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901627#comment-16901627 ] Andrew Purtell commented on HBASE-22623: I also would like to understand what will be “messed up”. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901624#comment-16901624 ] Geoffrey Jacoby commented on HBASE-22623: - [~Apache9] - could you please go into more detail on how exposing a mutable WALEdit to a coproc could "mess things up"? The comment in the code says something similar, but doesn't give any example or principle explaining why this is so, and I'd like to understand the argument. I mean, the coproc could have a bug or malicious code which corrupts the WALEdit, but any mutation pipeline coproc could do that by corrupting the mutation object. Any read pipeline coproc hook could have buggy or malicious code that replaced the scanner with one that returned nonsense. I get keeping operation-level coprocessors away from process-level objects and services, but WALEdit is an operation-level object just like mutations and scanners are. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901612#comment-16901612 ] Duo Zhang commented on HBASE-22623: --- The problem here is the WALEdit, it has set methods which could mess things up. The WALKey is already an interface so should be fine. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901311#comment-16901311 ] Andrew Purtell commented on HBASE-22623: Thanks Anoop. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901298#comment-16901298 ] Anoop Sam John commented on HBASE-22623: Ping [~Apache9]. He did most of the CP cleanup stuff for 2.0 > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16901262#comment-16901262 ] Andrew Purtell commented on HBASE-22623: The explicit intent of the new hook is to modify or decorate the objects going into the WAL. That is at least WALKey, but a WAL entry is built from two objects: the WALKey, and the WALEdit. The hook should address both, otherwise we are leaving an obvious gap. We have been in a position before where CP hooks needed to be modified because they didn't quite expose what was necessary for the CP application to change. I think we can avoid a fairly obvious gap here. The second question is why a WALEdit is immutable. My guess is it is only because there has never been a need to modify one after construction. We don't need to change WALEdit, but one of the parameters of this hook should carry the WALEdit, if only to make its state visible to the coprocessor. That said, I have no objection to removing the annotation from WALEdit's add method. Coprocessors should be able to change state in the agreed upon scope. In this issue I see us trying to build consensus that WALKey and WALEdit are useful inputs to a coprocessor which wants to decorate a WAL entry just before commit. There is a real use case for this in Phoenix. (Change capture device.) Unless there is an objection I think we have that consensus per the three way conversation between Geoffrey, Anoop, and myself. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16900800#comment-16900800 ] Anoop Sam John commented on HBASE-22623: The CP cleanup was a larger activity. Say some CP API is getting a param type of HRegion/HStore, possibly it can invoke some methods an get things messy! Also the BC is another imp aspect. The CPs will have to suffer going from one version to other as it used some HBase internal APIs. CP used it because we gave them a clear way for using it. The CP cleanup in 2.0 aimed to shut this down. (I believe Phoenix is the max suffered because of this internal API changes) Seeing the annotation at WALEdit and at the add() methods, it is clear that the thinking then was to use it in Immutable way. But in above comment I raised a Q that in existing CP API where we expect to create a WALEdit and pass it back, there itself how a CP can do that with a private exposed API been used. So ya either we should remove that private annotation or should give a Factory way of create WALEdit (For the existing usage itself) > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16900506#comment-16900506 ] Andrew Purtell commented on HBASE-22623: bq. WALEdit preWALAppend(WALKey, WALEditSuperInterfaceWithoutAddMethods edit) :-) I don't think immutable WALedits buy us anything, but I was not the one who did the refactor of the WAL interface so don't have the context. [~busbey] might you point us in the right direction? In my opinion we decorate WALedits with special cells, the compaction marker etc., and a coprocessor may want to do this too. Although it does not do so today I am thinking especially of Phoenix. We did consider special cells instead of WALkey attributes for HBASE-22622. Now that we are adding a hook for decorating the key, we should include coverage for decorating/mutating the edit as well, or I fear this will be a miss later when someone needs it. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16900145#comment-16900145 ] Geoffrey Jacoby commented on HBASE-22623: - So, I see that the immutability of the Region-created WALEdit is an explicit goal of the current code, but what I'm trying to understand is _why_. Immutability can be a wonderful thing; it lets you write idempotent code along functional principles where code has no side effects, just an output. But it seems to me the entire point of coprocessors _is to have side effects_. I'm sure we could come up with an indirect way to accomplish the adding of cells -- maybe something like WALEdit preWALAppend(WALKey, WALEditSuperInterfaceWithoutAddMethods edit), where HRegion then takes the returned new WALEdit and combines its Cells with the Cells of the existing WALEdit -- but I don't see what the added complexity achieves. It would just be more object churn for the garbage collector. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16899856#comment-16899856 ] Anoop Sam John commented on HBASE-22623: As part of CP clean up for 2.0 I believe we have added this Private annotation for add() methods. The idea is, even if we pass WALEdit type itself to come CPs, we dont want the CPs to change the state of that Object. One can use getters only. But again considering this class alone, it raises 2 questions now 1. There is a public method add(Map> familyMap) in WALEdit which can do same work as that of add() APIs. It was a miss for adding Private annotation for this method? Also there is a getCells() API which return the same 'cells' data structure. One can change the state of the object by dealing with this List. 2. There are CP APIs around batchMutate() which gets MiniBatchOperationInProgress param. This CP API allows a CP user to pass new Cells to be written to WAL for each of the mutations in the batch. That can be done using the API setWalEdit(int index, WALEdit walEdit) This means the CP has to create the WALEdit object. And then comes the Q how to add Cells to it? I can not see other static util API or a builder to create a new WALEdit with some cells. Said that already wrt the WALEdit we miss some thing Also for batchMutate() API only we have the CP APIs which helps to pass a CP defined WALEdit for the Mutations. (Those WALEdit cells from CP will get combined with Mutation's data cells and get written to WAL). For APIs like append/increment such a way is missing. So I am +1 for the idea of the new CP API allowing Cells to be added to WALEdit and so to WAL. (Only thing is with batchMutate() user needs to be careful. It will give 2 CP calls both allowing the same). But we better do not expose the Region built WALEdit and give a chance for the CPs to change its state directly. Can we think of a way like the MiniBatchOperationInProgress doing way? That the CP can some way pass it extra cells/WALEdit and we combine that along with the Mutation cells while write to WAL? Note : Existing issue with how a CP to create new WALEdit with Cells in it for adding to MiniBatchOperationInProgress still there. May be to handle in another Jira. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16899716#comment-16899716 ] Geoffrey Jacoby commented on HBASE-22623: - The current patch up for evaluation at PR 390 creates a coprocessor hook "void preWALAppend(WALKey key, WALEdit edit)", in which WALKey remains mostly immutable but has an extra addExtendedAttribute method in order to add annotations. At [~apurtell]'s suggestion, WALEdit is also being added, since it's likely to be useful. I removed the InterfaceAudience.Private from the two WALEdit.add() methods and also removed the comments that counsel against exposing the class to coprocessors that might want to modify it. The comments do not specify _why_ this was an explicit policy, which I'd like to understand, since modifying a WALEdit seems to me no more risky than anything else you can do with a coproc. :-) Bringing the discussion back over to the JIRA at [~anoop.hbase]'s suggestion. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[
https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16886410#comment-16886410
]
Andrew Purtell commented on HBASE-22623:
{quote}instrumentation for the two code paths could be done in separate JIRAs,
a new one for Append + Increment and this one for Batch Mutate. Both would need
to be done to get full coverage, and we'd want them in the same release
{quote}
Yes it could be done this way too.
I'm not making my mind up about this or anything ahead of seeing a patch to
evaluate. Let's consider the code.
> Add RegionObserver coprocessor hook for preWALAppend
>
>
> Key: HBASE-22623
> URL: https://issues.apache.org/jira/browse/HBASE-22623
> Project: HBase
> Issue Type: New Feature
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Fix For: 3.0.0, 1.5.0, 2.3.0
>
>
> While many coprocessor hooks expose the WALEdit to implementing coprocs,
> there aren't any that expose the WALKey before it's created and added to the
> WALEntry.
> It's sometimes useful for coprocessors to be able to edit the WALKey, for
> example to add extended attributes using the fields to be added in
> HBASE-22622.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[
https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16886408#comment-16886408
]
Andrew Purtell commented on HBASE-22623:
{quote}The signatures for the Append/Increment coprocs and the Batch mutation
coprocs are different enough that I'm somewhat skeptical we could make a single
hook that would work for both
{quote}
So don't.
The proposal is a new hook at every call site of WAL#append: "Add
RegionObserver coprocessor hook for preWALAppend"
> Add RegionObserver coprocessor hook for preWALAppend
>
>
> Key: HBASE-22623
> URL: https://issues.apache.org/jira/browse/HBASE-22623
> Project: HBase
> Issue Type: New Feature
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Fix For: 3.0.0, 1.5.0, 2.3.0
>
>
> While many coprocessor hooks expose the WALEdit to implementing coprocs,
> there aren't any that expose the WALKey before it's created and added to the
> WALEntry.
> It's sometimes useful for coprocessors to be able to edit the WALKey, for
> example to add extended attributes using the fields to be added in
> HBASE-22622.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16886378#comment-16886378 ] Geoffrey Jacoby commented on HBASE-22623: - What I meant is that for ease of evaluation and to keep patches at a modest size, instrumentation for the two code paths could be done in separate JIRAs, a new one for Append + Increment and this one for Batch Mutate. Both would need to be done to get full coverage, and we'd want them in the same release. (Phoenix does use Append and Increment in a few cases, such as atomic upserts.) I can also combine them in this JIRA, which it sounds like you'd prefer and would also be fine. The signatures for the Append/Increment coprocs and the Batch mutation coprocs are different enough that I'm somewhat skeptical we could make a single hook that would work for both, and doing the batch mutation instrumentation through the MiniBatchOperationInProgress eliminates the need for new coprocessor hooks on that path, and limiting those to a minimum (such as the seemingly-unavoidable ones on Append and Increment) seems good to me. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[
https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16886361#comment-16886361
]
Andrew Purtell commented on HBASE-22623:
{quote}They're mostly unrelated changes though, so it would either work to do
them together in this JIRA or separately.
{quote}
I don't follow that. My understanding of the intent here is to intercept
control of execution just prior to append of WAL entries to the log for
potential modification or decoration. If the solution misses some code paths
doesn't it fall short of the goal?
> Add RegionObserver coprocessor hook for preWALAppend
>
>
> Key: HBASE-22623
> URL: https://issues.apache.org/jira/browse/HBASE-22623
> Project: HBase
> Issue Type: New Feature
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Fix For: 3.0.0, 1.5.0, 2.3.0
>
>
> While many coprocessor hooks expose the WALEdit to implementing coprocs,
> there aren't any that expose the WALKey before it's created and added to the
> WALEntry.
> It's sometimes useful for coprocessors to be able to edit the WALKey, for
> example to add extended attributes using the fields to be added in
> HBASE-22622.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16886347#comment-16886347 ] Geoffrey Jacoby commented on HBASE-22623: - I think Append and Increment use a different code path (see HRegion.doDelta) and so would have to be instrumented a different way, perhaps with extra coprocessor hooks, one for Increment and one for Append. (The existing Append and Increment pre-hooks don't take a convenient "carrier" object like the batch mutations do, and already use their return values for other purposes.) checkAndXXX seems to eventually go down the same batch mutation path (see HRegion:4287) so the MiniBatchOperationInProgress solution above would work for them. They're unrelated changes though, so it > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > Fix For: 3.0.0, 1.5.0, 2.3.0 > > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[
https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16886322#comment-16886322
]
Andrew Purtell commented on HBASE-22623:
{quote} MiniBatchOperationInProgress object that gets created at the start of a
batch Put or Delete operation (doMiniBatchMutate), and which gets passed to
each subsequent coprocessor in the batch mutation pipeline. Both the main
pipeline and the coprocs would be able to read or write WALKey attributes to
the MiniBatchOperationInProgress, and all that has to change in the main
pipeline is an additional optional parameter on doWALAppend (which is private)
to instrument the WALKeyImpls it creates.
{quote}
Does this cover all cases where a WAL entry might be appended to the WAL?
Thinking of append, increment, and checkAndXXX operations.
> Add RegionObserver coprocessor hook for preWALAppend
>
>
> Key: HBASE-22623
> URL: https://issues.apache.org/jira/browse/HBASE-22623
> Project: HBase
> Issue Type: New Feature
>Reporter: Geoffrey Jacoby
>Assignee: Geoffrey Jacoby
>Priority: Major
> Fix For: 3.0.0, 1.5.0, 2.3.0
>
>
> While many coprocessor hooks expose the WALEdit to implementing coprocs,
> there aren't any that expose the WALKey before it's created and added to the
> WALEntry.
> It's sometimes useful for coprocessors to be able to edit the WALKey, for
> example to add extended attributes using the fields to be added in
> HBASE-22622.
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16885696#comment-16885696 ] Geoffrey Jacoby commented on HBASE-22623: - Upon looking more closely, it doesn't seem like my idea above of using ObserverContext to pass state to be added to the WALKey will work, because the write path in HRegion doesn't have access to the ObserverContext. So, new plan: there's a MiniBatchOperationInProgress object that gets created at the start of a batch Put or Delete operation (doMiniBatchMutate), and which gets passed to each subsequent coprocessor in the batch mutation pipeline. Both the main pipeline and the coprocs would be able to read or write WALKey attributes to the MiniBatchOperationInProgress, and all that has to change in the main pipeline is an additional optional parameter on doWALAppend (which is private) to instrument the WALKeyImpls it creates. Existing coprocessors won't have to change unless they want to take advantage of the new feature, and there are no new interface methods to implement. > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.14#76016)
[jira] [Commented] (HBASE-22623) Add RegionObserver coprocessor hook for preWALAppend
[ https://issues.apache.org/jira/browse/HBASE-22623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16880747#comment-16880747 ] Geoffrey Jacoby commented on HBASE-22623: - As an alternative to adding yet-another-new-coprocessor hook, or changing the behavior of an old one, how about this? Complete HBASE-18127, which adds a notion of OperationContext to the ObserverContext for the mutation pipeline. Then add a new field to the OperationContext, WALAttributeMap, which would then allow for _any_ coprocessor hook (before WAL committal) to annotate the WALKey. Then the WAL writing code would just read from the OperationContext and annotate the WALKey appropriately. No publicly overridable code would ever get anywhere near a protobuf, and the only interface that would change would be the HBASE-18127 OperationContext one that has never been released. (This would leave as future work the more general cases of HBASE-18127, such as passing information from a RegionObserver to a WALObserver, which prevents use of thread local storage.) [~apurtell] [~anoop.hbase][~abhishek.chouhan][~stack] > Add RegionObserver coprocessor hook for preWALAppend > > > Key: HBASE-22623 > URL: https://issues.apache.org/jira/browse/HBASE-22623 > Project: HBase > Issue Type: New Feature >Reporter: Geoffrey Jacoby >Assignee: Geoffrey Jacoby >Priority: Major > > While many coprocessor hooks expose the WALEdit to implementing coprocs, > there aren't any that expose the WALKey before it's created and added to the > WALEntry. > It's sometimes useful for coprocessors to be able to edit the WALKey, for > example to add extended attributes using the fields to be added in > HBASE-22622. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
