yurongfu commented on code in PR #8792:
URL: https://github.com/apache/hbase/pull/8792#discussion_r4493161985
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java:
##########
@@ -148,6 +148,14 @@ protected Flow executeFromState(final MasterProcedureEnv
env, TruncateTableState
break;
case TRUNCATE_TABLE_ADD_TO_META:
regions = CreateTableProcedure.addTableToMeta(env, tableDescriptor,
regions);
+ // Reconcile the namespace quota cache with the new region layout
(HBASE-30483).
+ // Replica regions do not count toward the namespace region quota.
This state has no
+ // rollback, so an IOException here makes the procedure retry the
whole state; both
+ // addTableToMeta and the absolute count update are idempotent, so
retrying is safe.
+ int defaultRegionCount =
+ (int)
regions.stream().filter(RegionReplicaUtil::isDefaultReplica).count();
+ ProcedureSyncWait.getMasterQuotaManager(env)
+ .setTableRegionCountForTruncate(getTableName(),
defaultRegionCount);
Review Comment:
Good question! Here's my reasoning for keeping the quota reconciliation
inline.
**On using a separate state:** I kept the update in
`TRUNCATE_TABLE_ADD_TO_META` because it is directly tied to the replacement
regions written to meta. Once `addTableToMeta()` completes, we have the new
region layout and can reconcile the namespace quota cache before proceeding to
region assignment.
I also considered the retry behavior. Both operations are idempotent:
`addTableToMeta()` can safely repeat the meta writes, and
`setTableRegionCountForTruncate()` sets an absolute count rather than applying
a delta. If the quota update throws an `IOException`, the existing procedure
retry mechanism can re-execute the state and bring both back to the expected
values. A separate state would provide a more explicit boundary and avoid
repeating the meta write, but I didn't think that warranted adding another
persisted state for this relatively small update.
There's also a similar pattern in
`RestoreSnapshotProcedure.prepareRestore()`, which performs namespace quota
reconciliation inline rather than introducing a separate state.
**On using a subprocedure:** The quota update is a synchronous, in-memory
operation on the Master, without independent scheduling or long-running work.
Since the namespace quota cache is reconstructed from meta during Master
initialization, I didn't see a need for separate procedure persistence or
recovery logic for this update. Introducing a child procedure seemed to add
more complexity than benefit.
The main trade-off I see is clarity: a dedicated state would make quota
reconciliation more explicit in the procedure flow and isolate its retries.
--
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]
