HBASE-29229 Throttles should support specific restrictions for atomic workloads #6866

rmdmattingly · 2025-03-28T20:20:45Z

Pretty straightforward, and well tested here imo: atomic requests can be uniquely expensive, and it would be nice to throttle them in isolation.

cc @ndimiduk @hgromer

rmdmattingly · 2025-03-28T20:21:16Z

Out of morbid curiosity, I'm going to request a review from copilot

Copilot

Pull Request Overview

This PR extends throttling support by introducing two new atomic quota types—atomic read size and atomic request number—to better isolate and restrict atomic workloads. Key changes include adding an extra boolean parameter to quota‐checking methods, updating rate limiter implementations to accommodate atomic quotas, and propagating these changes through tests, protocol buffers, and exception handling.

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestDefaultOperationQuota.java	Updated test calls to include the new isAtomic parameter.
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestDefaultAtomicQuota.java	Introduced new tests for atomic throttling behavior.
hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestAtomicReadQuota.java	Modified tests to use the updated quota setup methods and added atomic quota verifications.
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java	Updated checkBatchQuota to forward the isAtomic flag.
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/TimeBasedLimiter.java	Added new RateLimiter instances for atomic quotas and updated the checkQuota method accordingly.
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/RpcQuotaManager.java	Modified method signature to pass the isAtomic flag.
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/RegionServerRpcQuotaManager.java	Updated quota checks to propagate the atomic flag based on action type.
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java	Introduced new constants for default atomic quotas.
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaLimiter.java and related quota classes	Updated method signatures to accommodate the extra atomic parameter.
hbase-protocol-shaded/src/main/protobuf/server/Quota.proto	Added new enum definitions for atomic throttling.
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java, ThrottleType.java, ThrottleSettings.java, RpcThrottlingException.java	Updated client-side handling of atomic quota types and associated error messages.

rmdmattingly · 2025-03-28T20:22:17Z

Wow, perfection on the first try 😄

rmdmattingly · 2025-03-29T00:45:46Z

Test failure is unrelated

ndimiduk

Scrolling through on mobile, so it's difficult to investigate details at the moment.

Should we also support a throttle based on atomic write volume? I think that atomic operations are not read/write symmetrical -- that is, we can have a CAS that reads a very small value but writes a very large value. It's unclear to me how we determine the read size and if we can distinguish that from write size.

If these are two distinct metrics, I think that we should support two distinct throttles. If they are recorded as a single metric, we should use a name that does not specify read or write, something like "atomic_bytes_processed".

rmdmattingly · 2025-03-30T17:44:43Z

Quotas have some precedent for calculating a best guess at a mutations write size via this helper, and it uses that to support write volume specific throttles. IMO we could add another limiter here based on atomic write volume

ndimiduk · 2025-03-30T17:56:46Z

Wow, perfection on the first try 😄

It ran but did it have you any actionable feedback? Looks like it just wrote a change summary.

rmdmattingly · 2025-03-30T17:58:48Z

Yeah it has no feedback:

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

Then I did a self-review and found a few things I wanted to cleanup, that imo it could have caught 😢

… workloads

rmdmattingly · 2025-03-31T16:41:39Z

I just pushed an update which includes an atomic write size throttle

rmdmattingly · 2025-03-31T16:42:26Z

hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaCache.java

+  void forceSynchronousCacheRefresh() {
+    refreshChore.chore();
+  }


This addition is a bit weird, but I noticed that many tests were flappy when running locally due to the chore missing its trigger. So this is a more forceful approach that improves the test reliability

ndimiduk

Thanks for covering the write case.

Apache-HBase · 2025-04-01T00:13:08Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 33s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+0 🆗	buf	0m 0s		buf was not available.
+0 🆗	buf	0m 0s		buf was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	hbaseanti	0m 0s		Patch does not have any anti-patterns.
			_ master Compile Tests _
+0 🆗	mvndep	0m 10s		Maven dependency ordering for branch
+1 💚	mvninstall	3m 41s		master passed
+1 💚	compile	5m 23s		master passed
+1 💚	checkstyle	1m 12s		master passed
+1 💚	spotbugs	5m 43s		master passed
+1 💚	spotless	0m 52s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 12s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 40s		the patch passed
+1 💚	compile	6m 0s		the patch passed
+1 💚	cc	6m 0s		the patch passed
+1 💚	javac	6m 0s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	1m 17s		the patch passed
+1 💚	spotbugs	5m 48s		the patch passed
+1 💚	hadoopcheck	17m 10s		Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚	hbaseprotoc	2m 15s		the patch passed
+1 💚	spotless	1m 11s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 41s		The patch does not generate ASF License warnings.
		66m 20s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6866/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#6866
Optional Tests	dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless cc buflint bufcompat hbaseprotoc
uname	Linux f1d23eb1590c 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `eb31063`
Default Java	Eclipse Adoptium-17.0.11+9
Max. process+thread count	85 (vs. ulimit of 30000)
modules	C: hbase-protocol-shaded hbase-client hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6866/3/console
versions	git=2.34.1 maven=3.9.8 spotbugs=4.7.3
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2025-04-01T03:28:48Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 43s		Docker mode activated.
-0 ⚠️	yetus	0m 3s		Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
			_ Prechecks _
			_ master Compile Tests _
+0 🆗	mvndep	0m 12s		Maven dependency ordering for branch
+1 💚	mvninstall	3m 55s		master passed
+1 💚	compile	2m 23s		master passed
+1 💚	javadoc	1m 9s		master passed
+1 💚	shadedjars	7m 10s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 13s		Maven dependency ordering for patch
+1 💚	mvninstall	4m 0s		the patch passed
+1 💚	compile	2m 12s		the patch passed
+1 💚	javac	2m 12s		the patch passed
+1 💚	javadoc	1m 10s		the patch passed
+1 💚	shadedjars	6m 55s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	0m 45s		hbase-protocol-shaded in the patch passed.
+1 💚	unit	1m 53s		hbase-client in the patch passed.
-1 ❌	unit	218m 15s	/patch-unit-hbase-server.txt	hbase-server in the patch failed.
		255m 59s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6866/3/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#6866
Optional Tests	javac javadoc unit compile shadedjars
uname	Linux ce3ae9c54725 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `eb31063`
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6866/3/testReport/
Max. process+thread count	6187 (vs. ulimit of 30000)
modules	C: hbase-protocol-shaded hbase-client hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6866/3/console
versions	git=2.34.1 maven=3.9.8
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

… workloads (#6866) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

… workloads (#6866) (#6881) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

… workloads (#6866) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

… workloads (#6866) (#6882) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

… workloads (#6866) (#6882) (#6883) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

… workloads (apache#6866) (apache#6882) (apache#6883) (will be in 2.6.3) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

…rictions for atomic workloads (apache#6866) (apache#6882) (apache#6883) (will be in 2.6.3) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

… workloads (apache#6866) (apache#6882) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

rmdmattingly requested review from Copilot and ndimiduk March 28, 2025 20:21

Copilot AI reviewed Mar 28, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

rmdmattingly force-pushed the HBASE-29229 branch 2 times, most recently from a95c453 to ad22d1d Compare March 29, 2025 21:31

This comment has been minimized.

Sign in to view

ndimiduk reviewed Mar 30, 2025

View reviewed changes

HBASE-29229 Throttles should support specific restrictions for atomic…

eb31063

… workloads

rmdmattingly force-pushed the HBASE-29229 branch from ad22d1d to eb31063 Compare March 31, 2025 16:41

rmdmattingly commented Mar 31, 2025

View reviewed changes

ndimiduk approved these changes Mar 31, 2025

View reviewed changes

rmdmattingly merged commit fd0803a into apache:master Apr 3, 2025
1 check failed

rmdmattingly deleted the HBASE-29229 branch April 3, 2025 14:34

rmdmattingly added a commit that referenced this pull request Apr 3, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

a7d6ffa

… workloads (#6866) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly added a commit that referenced this pull request Apr 3, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

f6da0be

… workloads (#6866) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly mentioned this pull request Apr 3, 2025

Backport "HBASE-29229 Throttles should support specific restrictions for atomic…" to branch-3 #6881

Merged

rmdmattingly added a commit that referenced this pull request Apr 3, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

bff3198

… workloads (#6866) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly mentioned this pull request Apr 3, 2025

Backport "HBASE-29229 Throttles should support specific restrictions for atomic…" to branch-2 #6882

Merged

rmdmattingly added a commit that referenced this pull request Apr 3, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

77612dc

… workloads (#6866) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly mentioned this pull request Apr 3, 2025

Backport "HBASE-29229 Throttles should support specific restrictions for atomic…" to branch-2.6 #6883

Merged

rmdmattingly added a commit that referenced this pull request Apr 7, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

945d272

… workloads (#6866) (#6881) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

rmdmattingly added a commit that referenced this pull request Apr 7, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

9e07f9a

… workloads (#6866) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly added a commit that referenced this pull request Apr 8, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

41e9341

… workloads (#6866) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly added a commit that referenced this pull request Apr 8, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

c098b08

… workloads (#6866) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly added a commit that referenced this pull request Apr 8, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

978e713

… workloads (#6866) Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>

rmdmattingly added a commit that referenced this pull request Apr 9, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

7231dce

… workloads (#6866) (#6882) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

rmdmattingly added a commit that referenced this pull request Apr 9, 2025

HBASE-29229 Throttles should support specific restrictions for atomic…

feb4b55

… workloads (#6866) (#6882) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Co-authored-by: Ray Mattingly <rmattingly@hubspot.com>

rmdmattingly mentioned this pull request Apr 9, 2025

Backport HBASE-29229 HubSpot/hbase#173

Merged

HBASE-29229 Throttles should support specific restrictions for atomic workloads #6866

HBASE-29229 Throttles should support specific restrictions for atomic workloads #6866

Uh oh!

Conversation

rmdmattingly commented Mar 28, 2025

Uh oh!

rmdmattingly commented Mar 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

rmdmattingly commented Mar 28, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

rmdmattingly commented Mar 29, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

ndimiduk left a comment

Choose a reason for hiding this comment

Uh oh!

rmdmattingly commented Mar 30, 2025

Uh oh!

ndimiduk commented Mar 30, 2025

Uh oh!

rmdmattingly commented Mar 30, 2025

Uh oh!

rmdmattingly commented Mar 31, 2025

Uh oh!

rmdmattingly Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

ndimiduk left a comment

Choose a reason for hiding this comment

Uh oh!

Apache-HBase commented Apr 1, 2025

Uh oh!

Apache-HBase commented Apr 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants