Quarantined bookies are never released early when they become healthy again

[lhotari]

BUG REPORT

Describe the bug

A quarantined bookie is only released when a fixed timer expires. Nothing releases it when the bookie becomes healthy again.

BookieWatcherImpl builds the quarantine set as a write-expiring cache:

this.quarantinedBookies = CacheBuilder.newBuilder()
        .expireAfterWrite(conf.getBookieQuarantineTimeSeconds(), TimeUnit.SECONDS)
        .removalListener(...)
        .build();

The only other way out is releaseAllQuarantinedBookies(), which BookKeeper.checkForFaultyBookies() calls solely when the metadata-driver health-check flag has been switched off — i.e. when the whole feature is disabled, not when a particular bookie recovers.

Entry into quarantine is comparatively cheap:

if (pool.errorCounter.getAndSet(0) >= bookieErrorThresholdPerInterval) {
    faultyBookies.add(pool.address);
}

(BookieClientImpl.getFaultyBookies()), evaluated every bookieHealthCheckIntervalSeconds.

So a bookie that exceeds the error threshold inside a single check interval — one long GC pause, one brief network blip — is excluded from ensemble placement for the whole bookieQuarantineTimeSeconds, with no way to re-admit it early even though the very next interval shows it clean. Deployments that raise the quarantine time to reduce flapping make the asymmetry worse: the penalty for one bad interval grows while the recovery signal is still ignored.

Quarantine is soft — newEnsemble() and replaceBookie() both retry with an empty exclusion set on BKNotEnoughBookiesException — so this does not hard-fail placement. But it does silently degrade it: the fallback ("Not enough healthy bookies available, using quarantined bookies") is logged only at DEBUG, so an operator sees the initial quarantine WARN and then nothing until expiry.

To Reproduce

1. Enable the bookie health check on a client.
2. Cause one bookie to return more than bookieErrorThresholdPerInterval errors within a single bookieHealthCheckIntervalSeconds window, then let it return to normal immediately.
3. Observe Bookie <id> has been quarantined because of read/write errors.
4. Observe that the bookie is excluded from new ensembles for the full bookieQuarantineTimeSeconds, and that Bookie <id> is no longer quarantined only appears when the timer expires — regardless of the bookie being healthy for the entire period.

Expected behavior

Release a bookie from quarantine once it demonstrates health — for example, invalidate its entry when a subsequent health-check interval finds its error count below the threshold, or require N consecutive clean intervals before re-admitting. The fixed timer would then be an upper bound rather than the only mechanism.

Additional context

Separate but adjacent, and easy to misconfigure: bookieQuarantineRatio reads like a cap on how much of the fleet may be quarantined, but it is a per-bookie, per-check probability:

for (BookieId faultyBookie : faultyBookies) {
    if (Math.random() <= bookieQuarantineRatio) {
        bookieWatcher.quarantineBookie(faultyBookie);
        ...

(BookKeeper.checkForFaultyBookies()). It staggers the decision across clients — which matches its documented intent — but it places no bound on how many bookies a single client can quarantine, and with a short check interval a persistently faulty bookie is quarantined within a couple of intervals anyway. A documentation clarification, or an actual cap, would prevent the setting from being relied on for something it does not do.