rmdmattingly opened a new pull request, #8618: URL: https://github.com/apache/hbase/pull/8618
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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
