JIRA: HBASE-30484

Description

A table space quota with the DISABLE violation policy disables the table when
its usage exceeds the limit. On the active Master, QuotaObserverChore
periodically recomputes each table's quota snapshot and transitions the table
between violation and observance.

For example, a table over its limit reaches state similar to:

hbase:quota snapshot:       inViolation, policy=DISABLE
chore in-memory snapshot:   inViolation, policy=DISABLE
table state:                DISABLED

When the operator raises the limit (or usage drops) so the table is no longer
in violation, QuotaObserverChore.updateTableQuota() previously performed the
transition in this order:

if (!currentSnapshot.equals(targetSnapshot)) {
  this.snapshotNotifier.transitionTable(table, targetSnapshot);
  // Update it in memory
  tableSnapshotStore.setCurrentState(table, targetSnapshot);
  ...
  if (!targetStatus.isInViolation()) {
    // In case of Disable SVP, we need to enable the table as it moves out of violation
    if (isDisableSpaceViolationPolicy(currPolicy, targetPolicy)) {
      QuotaUtil.enableTableIfNotEnabled(conn, table);
    }
    ...

Both the persisted snapshot (transitionTable() writes to hbase:quota) and
the in-memory snapshot (setCurrentState()) are advanced before the table is
enabled. QuotaUtil.enableTableIfNotEnabled() calls Admin.enableTable(),
which can fail before an enable procedure is submitted — for example, a
failing preEnableTable coprocessor hook or a failed quota-table read in
HMaster.enableTable(). The exception propagates to chore(), which logs:

Failed to process quota reports and update quota state. Will retry.

Because both snapshots were already advanced, the failure leaves this state:

hbase:quota snapshot:       notInViolation (persisted)
chore in-memory snapshot:   notInViolation
table state:                DISABLED

On the next chore run, getTargetState() computes the same non-violating
snapshot, so currentSnapshot.equals(targetSnapshot) is true and
updateTableQuota() takes the no-op branch ("remains in observance of
quota"). The "Will retry" log message only means the chore runs again; the
failed table operation is never retained or retried. The table stays
DISABLED indefinitely, and client operations fail:

org.apache.hadoop.hbase.TableNotEnabledException: <table> is disabled

The insufficient-region-reports path in _chore() has the same ordering: a
table in "limbo" moving out of a DISABLE violation calls
setCurrentState() before enableTableIfNotEnabled(), with the same
consequence on failure. The disable direction (moving into violation) is
symmetric: a failed disableTableIfNotDisabled() leaves the table enabled
while both snapshots already record the violation.

Solution

Keep the write to hbase:quota first — HMaster.enableTable consults the
persisted snapshot, and transitionTable() is a plain idempotent Put, so
re-running it on retry is safe. Move the in-memory setCurrentState() call
after the table enable/disable operation, to the end of the transition branch:

if (!currentSnapshot.equals(targetSnapshot)) {
  this.snapshotNotifier.transitionTable(table, targetSnapshot);
  ...
  if (!targetStatus.isInViolation()) {
    if (isDisableSpaceViolationPolicy(currPolicy, targetPolicy)) {
      QuotaUtil.enableTableIfNotEnabled(conn, table);
    }
    ...
  } else {
    ...
  }
  // Update it in memory only after the table state change succeeded, so that a failed
  // enable/disable leaves the in-memory state unchanged and the next chore run retries it.
  tableSnapshotStore.setCurrentState(table, targetSnapshot);
}

If enableTableIfNotEnabled() / disableTableIfNotDisabled() throws, the
IOException propagates before setCurrentState(), so the old in-memory
snapshot retains the pending transition. The next chore run sees
currentSnapshot != targetSnapshot, re-enters the branch, and retries the
table operation. The helpers already suppress TableNotDisabledException,
TableNotEnabledException, and TableNotFoundException, so a retry against a
table that has since reached the requested state is a no-op.

The insufficient-region-reports path is reordered the same way:
transitionTable() → enableTableIfNotEnabled() → setCurrentState().

updateNamespaceQuota() is unchanged: its setCurrentState() is preceded
only by idempotent snapshot writes, with no table enable/disable on that path.

This fix covers retries while the same Master retains its in-memory state.
Recovery across a Master restart in the window between the persisted snapshot
write and the enable operation would require a durable record of the pending
table-state change and is out of scope.

Tests

Added:

• TestQuotaObserverChore.testRetryTableEnableOnFailure
• TestQuotaObserverChore.testRetryTableDisableOnFailure

Both are small mock-based unit tests (no mini-cluster). The enable test
performs this sequence:

1. Constructs a real QuotaObserverChore with a mocked Connection returning
   a mocked Admin, and a mocked SpaceQuotaSnapshotNotifier.
2. Calls initializeSnapshotStores() so the real TableQuotaSnapshotStore is
   wired to the chore's in-memory snapshot map.
3. Seeds the current snapshot as in violation with the DISABLE policy and
   builds a non-violating target snapshot.
4. Stubs Admin.enableTable() to throw an IOException on the first call and
   succeed on the second.
5. Calls updateTableQuota() and asserts it throws IOException, that the
   in-memory snapshot is still the violating one, and that enableTable() was
   invoked exactly once.
6. Calls updateTableQuota() again with the same current/target snapshots
   (what the next chore run would compute) and asserts no exception, that
   enableTable() was invoked a second time, and that the in-memory snapshot
   has advanced to the target.

testRetryTableDisableOnFailure is symmetric: it injects one failure from
Admin.disableTable() while moving into a DISABLE violation and verifies
the disable is retried before the snapshot advances.

Without the fix, the assertion in step 5 fails: the in-memory snapshot has
already advanced to the non-violating target before the injected enable
failure, which is exactly the state that caused the production chore to skip
the retry and leave the table permanently disabled.

TestQuotaObserverChoreRegionReports (mini-cluster, covering the
insufficient-region-reports path) continues to pass with the reorder.