PHOENIX-5860 Throw exception which region is closing or splitting when delete data by wangchao316 · Pull Request #874 · apache/phoenix

wangchao316 · 2020-09-06T02:42:08Z

No description provided.

…n delete data

wangchao316 · 2020-09-08T03:41:13Z

@joshelser @gjacoby126 @swaroopak , hello , could you please review this?
Thanks

stoty · 2020-09-28T17:34:29Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 0s	Docker mode activated.
-1 ❌	docker	5m 35s	Docker failed to build yetus/phoenix:871ed211e.

Subsystem	Report/Notes
GITHUB PR	#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.

…n delete data

wangchao316 · 2020-10-09T01:54:33Z

@joshelser , @stoty ，hello, I alter this code that postRollBackSplit. could you please review this?

yanxinyi · 2020-11-14T04:38:09Z

Hi @wangchao316 , the logic looks good to me, do you have anything to add @joshelser ?

wangchao316 · 2020-11-25T08:09:04Z

@joshelser ，hello， do you have anything to add?

joshelser · 2020-12-01T01:05:55Z

(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.

virajjasani · 2020-12-01T07:32:32Z

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?

virajjasani · 2020-12-01T09:49:27Z

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

chrajeshbabu · 2020-12-01T09:58:47Z

+1. It's really required to avoid issues on split rollback cases.

stoty · 2020-12-01T10:24:10Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	4m 20s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
		_ 4.x Compile Tests _
+1 💚	mvninstall	11m 12s	4.x passed
+1 💚	compile	0m 55s	4.x passed
+1 💚	checkstyle	0m 39s	4.x passed
+1 💚	javadoc	0m 44s	4.x passed
+0 🆗	spotbugs	2m 54s	phoenix-core in 4.x has 950 extant spotbugs warnings.
		_ Patch Compile Tests _
+1 💚	mvninstall	5m 26s	the patch passed
+1 💚	compile	0m 56s	the patch passed
+1 💚	javac	0m 56s	the patch passed
-1 ❌	checkstyle	0m 38s	phoenix-core: The patch generated 3 new + 364 unchanged - 0 fixed = 367 total (was 364)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 43s	the patch passed
+1 💚	spotbugs	3m 11s	the patch passed
		_ Other Tests _
-1 ❌	unit	136m 2s	phoenix-core in the patch failed.
+1 💚	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	#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.

wangchao316 · 2020-12-01T11:07:05Z

+1. It's really required to avoid issues on split rollback cases.

Thanks, can you please merge?

stoty · 2020-12-01T13:17:12Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 32s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
		_ 4.x Compile Tests _
+1 💚	mvninstall	11m 0s	4.x passed
+1 💚	compile	0m 55s	4.x passed
+1 💚	checkstyle	0m 38s	4.x passed
+1 💚	javadoc	0m 44s	4.x passed
+0 🆗	spotbugs	2m 53s	phoenix-core in 4.x has 950 extant spotbugs warnings.
		_ Patch Compile Tests _
+1 💚	mvninstall	5m 26s	the patch passed
+1 💚	compile	0m 57s	the patch passed
+1 💚	javac	0m 57s	the patch passed
-1 ❌	checkstyle	0m 38s	phoenix-core: The patch generated 5 new + 362 unchanged - 2 fixed = 367 total (was 364)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 46s	the patch passed
+1 💚	spotbugs	2m 59s	the patch passed
		_ Other Tests _
-1 ❌	unit	140m 21s	phoenix-core in the patch failed.
+1 💚	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	#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.

ChinmaySKulkarni

+1

PHOENIX-5860Throw exception which region is closing or splitting whe…
2c95ead
…n delete data

joshelser reviewed Sep 9, 2020
View reviewed changes

Comment threadphoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java Outdated

PHOENIX-5860Throw exception which region is closing or splitting whe…
5e842ed
…n delete data

ChinmaySKulkarni approved these changes Dec 1, 2020
View reviewed changes

Uh oh!

Conversation

wangchao316 commented Sep 6, 2020

Uh oh!

wangchao316 commented Sep 8, 2020

Uh oh!

Uh oh!

stoty commented Sep 28, 2020

Uh oh!

wangchao316 commented Oct 9, 2020

Uh oh!

yanxinyi commented Nov 14, 2020

Uh oh!

wangchao316 commented Nov 25, 2020

Uh oh!

joshelser commented Dec 1, 2020

Uh oh!

virajjasani commented Dec 1, 2020

Uh oh!

virajjasani commented Dec 1, 2020

Uh oh!

chrajeshbabu commented Dec 1, 2020

Uh oh!

stoty commented Dec 1, 2020

Uh oh!

wangchao316 commented Dec 1, 2020

Uh oh!

stoty commented Dec 1, 2020

Uh oh!

ChinmaySKulkarni left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants