Repository navigation
[fix][broker] Avoid load shedding and metadata writes from a former leader - #26253
Conversation
…eader Recheck leader state before load-shedding side effects and metadata writes so a task that was already running stops after leadership changes.
ef5750a to
d5e269e
Compare
|
Leader can still change between the isLeader() check and the actual unload, so this cannot fully prevent a former leader from triggering an unload. |
|
Thanks. This PR intentionally provides only a best-effort guard and does not address the narrow race between the leadership check and the unload itself. Providing a strong guarantee would require a broader fencing design, which is out of scope for this focused change. |
lhotari
left a comment
lhotari
left a comment
There was a problem hiding this comment.
The best-effort leadership checks look appropriate for the stated scope. I agree with the existing metrics-accounting comment: please preserve broker accounting and publish the completed unloads when leadership loss stops a partially completed run. A regression case that loses leadership after the first successful unload would cover this. This request concerns the reported metrics; stopping further unloads is correct.
Assisted-by: Codex
lhotari
left a comment
lhotari
left a comment
There was a problem hiding this comment.
LGTM. The leadership-loss paths now preserve accounting for completed unloads and publish the metrics before stopping. The added regression test covers leadership loss after the first successful unload and verifies both published totals. This addresses my previous concern.
Preserve leadership checks and unload-attempt completion cleanup, with assertions covering cleanup after leadership loss.
|
Merged The resolution preserves the leadership checks together with master's Validation: |
…eader (#26253) Co-authored-by: Lari Hotari <[email protected]> (cherry picked from commit af2c40d)
…eader (#26253) Co-authored-by: Lari Hotari <[email protected]> (cherry picked from commit af2c40d)
4 participants
Motivation
Load-balancer tasks are started only while a broker is the leader. However, cancelling a scheduled task does not stop an invocation that is already in progress. If leadership changes during such an invocation, the former leader can still initiate bundle unloading or persist load-balancing metadata.
Modifications
Verifying this change
This change added tests and can be verified as follows:
./gradlew :pulsar-broker:test -PtestGroups=broker -PexcludedTestGroups='' --tests org.apache.pulsar.broker.loadbalance.impl.ModularLoadManagerImplTest --console=plain./gradlew :pulsar-broker:checkstyleMain :pulsar-broker:checkstyleTest --console=plainDoes this pull request potentially affect one of the following parts: