| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR prevents futile autorecovery loops for ledgers with writeQuorumSize=1 by skipping failed-bookie underreplication tasks (and cleaning up historical ones), while adding dedicated operational metrics and regression tests.
Changes:
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file| File | Description |
|---|---|
| site3/website/docs/admin/autorecovery.md | Documents skipped single-replica failed-bookie tasks and the new metrics. |
| bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestReplicationWorker.java | Adds worker regression tests for historical cleanup, placement-policy preservation, and metrics. |
| bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorBookieCheckTaskTest.java | Adds auditor regression tests for single-replica filtering, deleted ledgers, and fail-open behavior. |
| bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationWorker.java | Introduces “skipped” outcome handling and historical cleanup guard for single-replica tasks. |
| bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/ReplicationStats.java | Adds new metric constants for auditor and worker skip counters. |
| bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/AuditorTask.java | Implements metadata-based filtering and adjusts published-ledger stats to exclude skipped items. |
| bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/AuditorStats.java | Adds auditor counter for single-replica skip events. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
|
@StevenLuMT Could you please take a look at this PR when you have a moment? Your review is highly appreciated. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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
AuditorTask.publishSuspectedLedgersAsync:
marked for normal recovery.
manager.
ledger id, quorum size, reason, and action for skipped publications.
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).
an empty replicaList continues through the existing replication path.
historical cleanup is not counted as replication success or failure. Add
NUM_SINGLE_REPLICA_UNDERREPLICATED_LEDGERS_SKIPPED for this path.
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:
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:
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.