You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/09/06 02:42:21 UTC
[GitHub] [phoenix] wangchao316 opened a new pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
wangchao316 opened a new pull request #874:
URL: https://github.com/apache/phoenix/pull/874
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] wangchao316 commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-705920881
@joshelser , @stoty ，hello, I alter this code that postRollBackSplit. could you please review this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-736362078
Precommit results are available: https://ci-hadoop.apache.org/job/PreCommit-PHOENIX-Build/229/testReport/
Looks good, failures don't seem relevant. +1(non-binding) for this PR.
Thanks @wangchao316
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] wangchao316 commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-705920881
@joshelser , @stoty ，hello, I alter this code that postRollBackSplit. could you please review this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] wangchao316 commented on a change in pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #874:
URL: https://github.com/apache/phoenix/pull/874#discussion_r497473813
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
##########
@@ -1430,4 +1430,17 @@ public InternalScanner run() throws Exception {
}
return s;
}
+
+ /**
+ * roll back after split failed, will isRegionClosingOrSplitting set false,
+ * and then write region will is available
+ * @param ctx
+ * @throws IOException
+ */
+ @Override
+ public void preRollBackSplit(ObserverContext<RegionCoprocessorEnvironment> ctx) throws IOException {
Review comment:
@joshelser ,hello, I alter this code that postRollBackSplit. could you please review this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] yanxinyi commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-727143377
Hi @wangchao316 , the logic looks good to me, do you have anything to add @joshelser ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] virajjasani commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-736281709
Thanks @joshelser for bringing this up. I just realized that we have this exact same issue reported sometime back this year with Phoenix 4.14.x + HBase 1.3.x for Delete queries.
Changes indeed look reasonable and simpler to understand.
I am confident about existence of the issue (not the fix though). And @wangchao316 seems to have been able to consistently repro this with minor changes in HBase split code by failing split all the times and let the flow through rollback of split, as per Jira comments.
However, let's trigger a QA run first, we don't have clear QA result on PR. If we don't see any issue, I am +1 for rolling out this change, feels worth shipping.
@gjacoby126 @ChinmaySKulkarni @apurtell Thought?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-736545208
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ 4.x Compile Tests _ |
| +1 :green_heart: | mvninstall | 11m 0s | 4.x passed |
| +1 :green_heart: | compile | 0m 55s | 4.x passed |
| +1 :green_heart: | checkstyle | 0m 38s | 4.x passed |
| +1 :green_heart: | javadoc | 0m 44s | 4.x passed |
| +0 :ok: | spotbugs | 2m 53s | phoenix-core in 4.x has 950 extant spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 26s | the patch passed |
| +1 :green_heart: | compile | 0m 57s | the patch passed |
| +1 :green_heart: | javac | 0m 57s | the patch passed |
| -1 :x: | checkstyle | 0m 38s | phoenix-core: The patch generated 5 new + 362 unchanged - 2 fixed = 367 total (was 364) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed |
| +1 :green_heart: | spotbugs | 2m 59s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 140m 21s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 39s | The patch does not generate ASF License warnings. |
| | | 171m 8s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | phoenix.query.MaxConcurrentConnectionsIT |
| | phoenix.end2end.AlterTableWithViewsIT |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/874 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile |
| uname | Linux 0aa40ae946ff 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | 4.x / 18b9f76 |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/4/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/4/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/4/testReport/ |
| Max. process+thread count | 6597 (vs. ulimit of 30000) |
| modules | C: phoenix-core U: phoenix-core |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/4/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-736382407
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 4m 20s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ 4.x Compile Tests _ |
| +1 :green_heart: | mvninstall | 11m 12s | 4.x passed |
| +1 :green_heart: | compile | 0m 55s | 4.x passed |
| +1 :green_heart: | checkstyle | 0m 39s | 4.x passed |
| +1 :green_heart: | javadoc | 0m 44s | 4.x passed |
| +0 :ok: | spotbugs | 2m 54s | phoenix-core in 4.x has 950 extant spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 26s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| -1 :x: | checkstyle | 0m 38s | phoenix-core: The patch generated 3 new + 364 unchanged - 0 fixed = 367 total (was 364) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 43s | the patch passed |
| +1 :green_heart: | spotbugs | 3m 11s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 136m 2s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 37s | The patch does not generate ASF License warnings. |
| | | 170m 58s | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | phoenix.end2end.index.LocalImmutableTxIndexIT |
| | phoenix.end2end.IndexBuildTimestampIT |
| | TEST-[RangeScanIT_0] |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/phoenix/pull/874 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile |
| uname | Linux 64dce7320f26 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | 4.x / 277b6fd |
| Default Java | Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt |
| unit | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/3/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt |
| Test Results | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/3/testReport/ |
| Max. process+thread count | 6709 (vs. ulimit of 30000) |
| modules | C: phoenix-core U: phoenix-core |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/3/console |
| versions | git=2.7.4 maven=3.3.9 spotbugs=4.1.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] stoty commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
stoty commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-700178585
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 0s | Docker mode activated. |
| -1 :x: | docker | 5m 35s | Docker failed to build yetus/phoenix:871ed211e. |
| Subsystem | Report/Notes |
|----------:|:-------------|
| GITHUB PR | https://github.com/apache/phoenix/pull/874 |
| Console output | https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-874/1/console |
| versions | git=2.17.1 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] wangchao316 commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-733539300
@joshelser ，hello， do you have anything to add?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] chrajeshbabu commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
chrajeshbabu commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-736367472
+1. It's really required to avoid issues on split rollback cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] joshelser commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
joshelser commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-736149905
(sorry for the wide ping)
@virajjasani @gjacoby126 @ChinmaySKulkarni @apurtell have any of your noticed this one in 1.x? The change seems reasonable to me, but I haven't kept up with the later 1.x HBase releases to have confidence in the issue and fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] wangchao316 commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-736428466
> +1. It's really required to avoid issues on split rollback cases.
Thanks, can you please merge?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] wangchao316 commented on a change in pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #874:
URL: https://github.com/apache/phoenix/pull/874#discussion_r486022598
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
##########
@@ -1430,4 +1430,17 @@ public InternalScanner run() throws Exception {
}
return s;
}
+
+ /**
+ * roll back after split failed, will isRegionClosingOrSplitting set false,
+ * and then write region will is available
+ * @param ctx
+ * @throws IOException
+ */
+ @Override
+ public void preRollBackSplit(ObserverContext<RegionCoprocessorEnvironment> ctx) throws IOException {
Review comment:
Thanks @joshelser , you are right, postRollBackSplit seems more correct to me . I will close this MR, and reopen a new MR, which use postRollBackSplit method.
HBase 2.x does not have this issue, because 2.x split failed, by SplitTableRegionProcedure rollbackState method , assign open region, so isRegionClosingOrSplitting is false.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] joshelser commented on a change in pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #874:
URL: https://github.com/apache/phoenix/pull/874#discussion_r533005013
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
##########
@@ -1430,4 +1430,17 @@ public InternalScanner run() throws Exception {
}
return s;
}
+
+ /**
+ * roll back after split failed, will isRegionClosingOrSplitting set false,
+ * and then write region will is available
+ * @param ctx
+ * @throws IOException
+ */
+ @Override
+ public void preRollBackSplit(ObserverContext<RegionCoprocessorEnvironment> ctx) throws IOException {
Review comment:
> HBase 2.x does not have this issue, because 2.x split failed, by SplitTableRegionProcedure rollbackState method , assign open region, so isRegionClosingOrSplitting is false.
Well, I think it's more accurate to say "HBase 2.x doesn't apply because splits are executed by the master". This coprocessor would never be invoked, running inside a RegionServer.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] joshelser commented on a change in pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #874:
URL: https://github.com/apache/phoenix/pull/874#discussion_r485821321
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java
##########
@@ -1430,4 +1430,17 @@ public InternalScanner run() throws Exception {
}
return s;
}
+
+ /**
+ * roll back after split failed, will isRegionClosingOrSplitting set false,
+ * and then write region will is available
+ * @param ctx
+ * @throws IOException
+ */
+ @Override
+ public void preRollBackSplit(ObserverContext<RegionCoprocessorEnvironment> ctx) throws IOException {
Review comment:
Do you want `preRollBackSplit` and not `postRollBackSplit`? If we're only about to rollback a split, I would say that, semantically, the region is still splitting until that operation is complete. postRollBackSplit seems more correct to me.
Also want to note: this method is only relevant for HBase 1.x.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [phoenix] wangchao316 commented on pull request #874: PHOENIX-5860 Throw exception which region is closing or splitting when delete data
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on pull request #874:
URL: https://github.com/apache/phoenix/pull/874#issuecomment-688600383
@joshelser @gjacoby126 @swaroopak , hello , could you please review this?
Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org