apache / apache/bookkeeper

Quarantined bookies are never released early when they become healthy again

Open
#4,879 0 comments 0 reactions 1 assignee Claimed by @lhotari View on GitHub
type/bug
Dominant language
Java
Stars
2k
Forks
976
Avg merge
6d 15h
Merged PRs (30d)
7

Description

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

```java
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:

```java
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 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 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:

```java
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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.