[PR] Revert "HBASE-30348 Throttles should work for system tables (excluding quotas, meta) (#8585) (#8590) (#8595)" [hbase]

["rmdmattingly (via GitHub)" <gi...@apache.org>]

rmdmattingly opened a new pull request, #8607:
URL: https://github.com/apache/hbase/pull/8607

   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: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Revert "HBASE-30348 Throttles should work for system tables (excluding quotas, meta) (#8585) (#8590) (#8595)" [hbase]

["rmdmattingly (via GitHub)" <gi...@apache.org>]

rmdmattingly merged PR #8607:
URL: https://github.com/apache/hbase/pull/8607


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