Radiancebobo opened a new pull request, #4873:
URL: https://github.com/apache/bookkeeper/pull/4873
Skip failed-bookie recovery tasks for ledgers with writeQuorumSize=1, and
clean up historical tasks without affecting placement-policy repairs.
Add dedicated metrics and regression tests for task filtering, lock release,
normal recovery retries, and cleanup failures.
Fix #4872
### Motivation
BookKeeper can create ledgers with `writeQuorumSize=1`. Every entry in such a
ledger has only one data source. If that Bookie is permanently lost, the
Auditor currently publishes an underreplication task, and the
`ReplicationWorker` repeatedly tries to read a source replica that cannot
exist. The retry loop consumes worker, client, and ZooKeeper resources and
creates noisy failure logs while obscuring ledgers that can actually be
recovered.
This change intentionally does not claim to recover or reconstruct lost data.
It stops an impossible automatic recovery path and exposes a dedicated metric
so operators can alert on the skipped condition and use backups or manual
recovery when data must be restored.
### Changes
* Filter failed-Bookie underreplication publication in
`AuditorTask.publishSuspectedLedgersAsync`:
* Read ledger metadata before creating the underreplication mark.
* Skip ledgers whose metadata has `writeQuorumSize == 1`.
* Keep metadata-read failures fail-open, so an unknown ledger is still
marked for normal recovery.
* Ignore the race where the ledger was deleted before publication.
* Count and size only ledgers actually submitted to the underreplication
manager.
* Add `NUM_SINGLE_REPLICA_LEDGERS_SKIPPED` in the Auditor scope and log the
ledger id, quorum size, reason, and action for skipped publications.
* Add a consumer-side guard in `ReplicationWorker` for historical tasks that
predate this change. After taking the task lock, a task is removed through
`markLedgerReplicated` when `writeQuorumSize == 1` and its `replicaList` is
non-empty (the failed-Bookie task shape).
* Preserve placement-policy repair tasks: a `writeQuorumSize == 1` task with
an empty `replicaList` continues through the existing replication path.
* Represent `SUCCESS`, `FAILED`, and `SKIPPED` worker outcomes separately so
historical cleanup is not counted as replication success or failure. Add
`NUM_SINGLE_REPLICA_UNDERREPLICATED_LEDGERS_SKIPPED` for this path.
* Add unit and integration coverage for single-replica filtering, deleted
ledgers, fail-open metadata errors, published-ledger statistics, historical
cleanup, placement-policy task preservation, and skipped-operation metrics.
### Compatibility and operational notes
No configuration, ZooKeeper schema, protobuf field, or public API is added.
The behavior is deterministic from ledger metadata and applies on upgrade.
Existing underreplication znodes are persistent, so the Worker-side guard is
required to converge historical failed-Bookie tasks. Tasks with an empty
`replicaList`, unreadable metadata, or a concurrent znode version change are
not forcibly removed by the single-replica cleanup guard. If metadata is
unreadable, the Worker falls back to the normal recovery path; that path may
remove the task if recovery completes successfully.
The skipped counters are operational signals, not recovery-success counters:
* `auditor.NUM_SINGLE_REPLICA_LEDGERS_SKIPPED`
* `replication_worker.NUM_SINGLE_REPLICA_UNDERREPLICATED_LEDGERS_SKIPPED`
These counters are cumulative skip events, not a deduplicated count of unique
ledger IDs. The same ledger may be counted again during a later audit or
retry.
### Verification
Tests run on the branch:
```text
mvn -pl bookkeeper-server -am -DskipTests=false \
-Dtest='AuditorBookieCheckTaskTest,TestReplicationWorker' \
-Dsurefire.failIfNoSpecifiedTests=false test
```
The command passes with 25 tests: 6 Auditor tests and 19 ReplicationWorker
tests. The full BookKeeper precommit command remains appropriate before
merge:
`mvn clean apache-rat:check install spotbugs:check`.
> ---
> In order to uphold a high standard for quality for code contributions,
Apache BookKeeper runs various precommit
> checks for pull requests. A pull request can only be merged when it passes
precommit checks.
>
> ---
> Be sure to do all the following to help us incorporate your contribution
> quickly and easily:
>
> Otherwise:
>
> - [x] Make sure the PR title is formatted like:
> `<Issue #>: Description of pull request`
> - [x] Replace `<Issue #>` in the title and `#TODO` above with the actual
Issue number.
> - [x] Add targeted regression and integration tests for the behavior
change.
> - [x] Run the full precommit command before merge: `mvn clean
apache-rat:check install spotbugs:check`.
>
> ---
--
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]
