yurongfu opened a new pull request, #8791:
URL: https://github.com/apache/hbase/pull/8791
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:
```text
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:
```java
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:
```text
Failed to process quota reports and update quota state. Will retry.
```
Because both snapshots were already advanced, the failure leaves this state:
```text
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:
```text
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:
```java
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.
--
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]
