lhotari opened a new issue, #4879:
URL: https://github.com/apache/bookkeeper/issues/4879
**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 <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:
```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.
--
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]
