You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "rmdmattingly (via GitHub)" <gi...@apache.org> on 2026/09/03 13:05:50 UTC
[PR] HBASE-30348 Throttles should work for system tables (excluding quotas, meta, namespace) [hbase]
rmdmattingly opened a new pull request, #8617:
URL: https://github.com/apache/hbase/pull/8617
https://issues.apache.org/jira/browse/HBASE-30348
Second attempt at HBASE-30348. The first was reverted because it deadlocked master startup on branch-2.x, as [~zhangduo] found. Opening as a draft to get CI results on master and branch-2 before asking for review.
### What changed since the reverted version
**`hbase:namespace` is now exempt as well as `hbase:meta` and `hbase:quota`.** This is the actual fix for the deadlock. `TableNamespaceManager` reads `hbase:namespace` during `initClusterSchemaService()`, which runs before `initQuotaManager()` creates `hbase:quota` — so throttling that table meant the read could wait on a quota system that could not yet answer. Every other table becomes throttleable, including the backup tables, which is what motivated the issue: `backup:system` reads could not be bounded by any quota.
**The initial quota cache load no longer runs inline on the RPC handler.** `QuotaCache#ensureInitialized` submits it to a single background thread and waits with a timeout, configurable via `hbase.quota.cache.initial.load.timeout.ms` and defaulting to ten seconds. A request that waits out the timeout proceeds with default quota state, which does not throttle, and the load is picked up once it completes.
This second part is defence in depth rather than the fix. The load reads `hbase:quota` *and* asks the master for cluster metrics via `getRegionServers()`/`getClusterMetrics()`, so any read of a non-exempt table arriving during master initialization could previously wait on the master indefinitely. It also means requests no longer queue on the monitor that `QuotaRefresherChore#chore` holds across those RPCs.
The load remains synchronous in effect, which preserves two existing behaviours that a fully asynchronous version broke: a quota that was just set is still visible to the next lookup (`TestQuotaCache`), and cluster scope factors are still computed before the first cluster scope check (`TestClusterScopeQuotaThrottle`).
### On the 3.x review
The hazard window is any read of a non-exempt table before `setInitialized(true)` in `finishActiveMasterInitialization`. On master the only such read I found is `TableNamespaceManager#loadNamespaceIntoCache`, which takes `loadFromNamespace()` — a scan of `hbase:namespace` — when `shouldLoadFromMeta()` returns false. That is reachable on a cluster upgraded from 2.x whose namespace migration has not completed; clusters created by 3.x have `NAMESPACE_FAMILY` in meta and take the safe `loadFromMeta()` path, which is why CI was green on branch-3 and branch-3.0 while branch-2 failed.
`hbase:acl` and `hbase:labels` are read from `postStartMaster()`, after initialization. `hbase:rsgroup` is read by `RSGroupInfoManagerImpl.RSGroupStartupWorker`, a daemon thread that loops on `isMasterRunning()` and retries, so it is off the initialization path. `hbase:canary`, `hbase:slowlog` and `hbase:replication` are not read in that window.
I have not reproduced the upgrade case, so that finding is reasoned from the code rather than observed.
### Testing
`TestQuotaUtil` is added, covering that `hbase:meta`, `hbase:quota` and `hbase:namespace` are exempt while the backup tables, the other `hbase` namespace tables, and user tables are not.
Locally, the `org.apache.hadoop.hbase.quotas` and `org.apache.hadoop.hbase.master` packages pass on both branches:
- master: 440 tests, 0 failures, 0 errors, 25 skipped (127 classes)
- branch-2.6: 439 tests, 0 failures, 0 errors, 26 skipped (129 classes)
Not yet covered: nothing exercises the timeout branch, since no test makes the load exceed the bound. A test that injects a hanging refresh and asserts the lookup returns within the timeout would cover it, and would also serve as a regression test for the deadlock itself. Happy to add that if it seems worthwhile.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30348 Throttles should work for system tables (excluding quotas, meta, namespace) [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #8617:
URL: https://github.com/apache/hbase/pull/8617#discussion_r4025623572
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaCache.java:
##########
@@ -118,12 +139,31 @@ public boolean isStopped() {
}
private void ensureInitialized() {
- if (!initialized) {
- synchronized (initializerLock) {
- if (!initialized) {
- refreshChore.chore();
- initialized = true;
- }
+ if (initialized) {
+ return;
+ }
+ synchronized (initializerLock) {
+ if (initialized) {
+ return;
+ }
+ if (initialLoad == null) {
+ initialLoad = initialLoadExecutor.submit(() -> refreshChore.chore());
+ }
+ try {
+ initialLoad.get(initialLoadTimeoutMs, TimeUnit.MILLISECONDS);
Review Comment:
Correct, I think we should just avoid throttling on the namespace table entirely. Meanwhile I still think it makes sense to remove the possibility of a deadlock in this codepath entirely to help future proof it from similar mistakes
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30348 Throttles should work for system tables (excluding quotas, meta, namespace) [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on PR #8617:
URL: https://github.com/apache/hbase/pull/8617#issuecomment-6200444341
Yeah, I like that idea — it's probably simpler than trying to manage a set of exclusions
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30348 Throttles should work for system tables (excluding quotas, meta, namespace) [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #8617:
URL: https://github.com/apache/hbase/pull/8617#issuecomment-6019218153
For me, I think we can define throttle as 'best effort', so you are free to set throttle on meta/namespace and other system tables, even for quota table itself, but if the quota data is not ready, we will consider as no throttle.
What do you think of this approach? Or we could allow some configuration based fallback rules for some tables?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30348 Throttles should work for system tables (excluding quotas, meta, namespace) [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #8617:
URL: https://github.com/apache/hbase/pull/8617#discussion_r4025371931
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaCache.java:
##########
@@ -118,12 +139,31 @@ public boolean isStopped() {
}
private void ensureInitialized() {
- if (!initialized) {
- synchronized (initializerLock) {
- if (!initialized) {
- refreshChore.chore();
- initialized = true;
- }
+ if (initialized) {
+ return;
+ }
+ synchronized (initializerLock) {
+ if (initialized) {
+ return;
+ }
+ if (initialLoad == null) {
+ initialLoad = initialLoadExecutor.submit(() -> refreshChore.chore());
+ }
+ try {
+ initialLoad.get(initialLoadTimeoutMs, TimeUnit.MILLISECONDS);
Review Comment:
So hwat is this used for?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30348 Throttles should work for system tables (excluding quotas, meta, namespace) [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on PR #8617:
URL: https://github.com/apache/hbase/pull/8617#issuecomment-5995759041
Hey @Apache9, any more thoughts here? I want to make sure we're in sync before we ship this given how it blew up in the prior attempt
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org