rmdmattingly opened a new pull request, #8605: URL: https://github.com/apache/hbase/pull/8605
Reverts the HBASE-30348 change on this branch while a corrected approach is worked out. See https://issues.apache.org/jira/browse/HBASE-30348 (reopened). [~zhangduo] found that the change deadlocks HMaster initialization on branch-2.x and has reverted it there. Reverting the 3.x/master lines as well, because the same hazard exists here on one path. ### The hazard, generally The danger is any read of a now-non-exempt table that happens **before `setInitialized(true)`** in `HMaster.finishActiveMasterInitialization()`. Until that point the master answers `PleaseHoldException`, so a RegionServer with a cold `QuotaCache` runs `ensureInitialized()` -> `refreshChore.chore()` synchronously on an RPC handler, and `updateQuotaFactors()` issues `getRegionServers()` back to the still-initializing master. That is circular. Ordering on master (offsets from the start of `finishActiveMasterInitialization`): ``` +200 waitForMetaOnline() hbase:meta - exempt +279 waitForNamespaceOnline() reads hbase:meta + in-memory assignment state only +283 initClusterSchemaService() <-- namespace load happens here +321 setInitialized(true) <-- hazard window closes +356 initQuotaManager() creates hbase:quota +420 postStartMaster() AccessController/VisibilityController touch acl/labels ``` ### Why 3.x is still affected `TableNamespaceManager.start()` -> `loadNamespaceIntoCache()` branches: ```java if (shouldLoadFromMeta()) { loadFromMeta(); // scans hbase:meta - exempt, safe } else { loadFromNamespace(); // scans hbase:namespace - NOT exempt after this change } ``` `shouldLoadFromMeta()` returns true when `migrationDone` (meta has `NAMESPACE_FAMILY`), when the namespace table is disabled, or when a `DisableTableProcedure` for it is pending. Otherwise `loadFromNamespace()` scans `hbase:namespace` at +283, inside the hazard window. On any cluster created by 3.x code, meta has `NAMESPACE_FAMILY` from the outset, so `migrationDone` is true and the safe branch is always taken. The unsafe branch is reachable only on a **cluster rolling-upgraded from 2.x whose namespace migration has not completed**. No test constructs that state, which is why CI was green on branch-3 and branch-3.0 while branch-2 failed. Note that `TableNamespaceManager` itself documents the timing dependency: *"since we are part of the master initialization work, so we can make sure that when reaching here, the master has not been marked as initialize yet."* ### What is not affected - `hbase:acl` and `hbase:labels` are read from `postStartMaster()` at +420, after `setInitialized(true)`. `initializeCoprocessorHost()` at +120 only loads classes and runs before meta is online. - `hbase:rsgroup` is read by `RSGroupInfoManagerImpl.RSGroupStartupWorker`, a daemon `Thread` that loops on `isMasterRunning()` and retries, so it is off the initialization critical path and tolerates transient failure. - `hbase:canary`, `hbase:slowlog`, and `hbase:replication` are not read in the pre-initialization window. To be clear about the evidence: the `loadFromNamespace()` exposure is reasoned from the code paths above and has **not** been reproduced against a real 2.x-to-3.x upgrade. ### Verification - The revert touches only the 4 files from the original commit - `hbase-server/.../quotas/` is byte-identical to its pre-change content - `mvn -pl hbase-server spotless:check` passes ### Next step The original problem stands: `backup:*` tables cannot currently be throttled, which is what motivated this. Rather than only widening the exempt list, the preferred direction is to make the quota path **fail open** when it cannot be served -- avoid the synchronous `refreshChore.chore()` on an RPC handler, and skip or tolerate `updateQuotaFactors()`'s admin RPC when the master is not yet initialized. That addresses the whole class rather than one instance of it, and would let the original change land safely. -- 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]
