fix(cluster): LeaderElector 實作 _watch_leader_key 偵測外部 DELETE#143
Open
csp0924 wants to merge 2 commits into
Open
fix(cluster): LeaderElector 實作 _watch_leader_key 偵測外部 DELETE#143csp0924 wants to merge 2 commits into
csp0924 wants to merge 2 commits into
Conversation
…並 demote `_watch_leader_key` 過去是 stub（body 為 `pass`），但 spec / docstring 明示 「如果 key 被刪除（非我們自己 resign），觸發 demotion」。實務上若 etcd operator 手動 `etcdctl del` election key、或 etcd partition 把 key 判給對方， 現在的 elector 完全偵測不到，必須等 keepalive 連續 `max_keepalive_failures` 次失敗（預設約 10s）才轉 demoted，期間 leader 仍認為自己是 leader → 雙寫 風險視窗放大。 本 PR 實作 watch loop： - `_resigning` 旗標區分 self-induced DELETE vs external DELETE，避免自身 resign 過程二次觸發 `on_demoted` 回呼 - 收到 external DELETE → 立即 `_handle_demotion()`，bounded latency - watch 連線異常 → 指數退避（0.1s → 5.0s 上限）重連，並透過 `_stop_event` 協作取消 - `asyncio.CancelledError` 路徑乾淨退出，配合 `_on_stop` 的 task cancel `_is_delete_event` 兼容 etcetra 的 `WatchEvent.event: WatchEventType` enum 與測試 mock 的 `.event_type: str` 兩種介面，不依賴 etcetra 內部結構。 測試補強： - `tests/cluster/test_election_watch.py`（4 案）：external DELETE → demote in bounded time、self resign 不重複觸發 demoted、PUT 事件不誤觸 demote、 watch 異常不 crash elector - `tests/cluster/test_election.py::_make_mock_client` 加入 `client.watch` 預設回傳「hang 到被 cancel」的 async generator，避免 leader-elected 後 watch task 拿到 default MagicMock 進入未定義 async iteration 行為導致 既有測試 stop() 卡死
There was a problem hiding this comment.
Pull request overview
This PR completes the missing leader-key watch behavior in LeaderElector so a leader can detect external deletion of the election key (e.g., manual etcdctl del, ACL changes, partitions) and demote promptly, reducing the dual-writer risk window.
Changes:
- Implement
LeaderElector._watch_leader_key()with external-DELETE detection, retry w/ exponential backoff, and a_resigningflag to suppress self-resign delete handling. - Add
_is_delete_event()to support both etcetra event shapes and test mocks. - Add new watch-focused tests and adjust existing mocks so leader watch tasks don’t cause
stop()hangs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
csp_lib/cluster/election.py |
Implements the leader watch loop, delete-event detection helper, and resign flag/backoff configuration. |
tests/cluster/test_election.py |
Updates the shared mock etcd client to provide a cancellable default watch() iterator. |
tests/cluster/test_election_watch.py |
Adds unit tests covering external DELETE demotion, self-resign behavior, PUT no-op, and watch retry on exceptions. |
Comments suppressed due to low confidence (2)
csp_lib/cluster/election.py:299
- 當 watch iterator 自行結束（async for 跑完）時，程式會立刻進入下一輪 while 並再次呼叫 client.watch()，但沒有任何等待/退避；若 watch 因錯誤快速結束，這裡會變成 tight loop 造成 CPU spin 與大量重連。建議把 iterator 結束也視為異常路徑套用 backoff（或至少 await stop_event/backoff 一次）再重試。
# watch iterator 結束（連線斷）— 重試
backoff = self._watch_backoff_initial
except asyncio.CancelledError:
tests/cluster/test_election_watch.py:114
- 這裡用 asyncio.get_event_loop().time() 與手寫 sleep 輪詢會在 Python 3.13+ 觸發/累積 DeprecationWarning（建議改用 asyncio.get_running_loop()），且 sleep-then-assert 較容易 flaky。此 repo 已提供 tests.helpers.wait_for_condition() 作為一致的輪詢 helper（例如 tests/manager/test_device_group.py），建議改用該 helper 來等待 on_demoted 被呼叫。
deadline = asyncio.get_event_loop().time() + 0.5
while asyncio.get_event_loop().time() < deadline:
if on_demoted.await_count > 0:
break
await asyncio.sleep(0.01)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+291
to
+296
| logger.warning( | ||
| f"Election key externally deleted, demoting: " | ||
| f"instance={self._config.instance_id}, key={election_key}" | ||
| ) | ||
| await self._handle_demotion() | ||
| return |
Comment on lines
+39
to
+43
| def _make_watch_queue(events: list[FakeWatchEvent], hang_forever: bool = True): | ||
| """建立一個 async iterator，依序 yield 給定的 events。 | ||
|
|
||
| hang_forever=True：yield 完所有事件後阻塞（模擬 etcd watch 長連線）。 | ||
| """ |
_watch_leader_key 與 _keepalive_loop 觸發 _handle_demotion 後僅 return， 但 keepalive task 仍依 _stop_event 為唯一終止條件運轉、campaign loop 也 卡在 _stop_event.wait() 無法重新進入競選，導致降級後持續產生 keepalive 噪音 / lease 續期，且 elector 永遠停在 LEADER 狀態無法 follower/candidate。 修法：新增獨立 _term_ended_event 表達「任期結束」語義（與「整體 stop」 分離）。_handle_demotion 設置該 event，campaign loop 透過 _wait_for_term_or_stop 從 leader 分支脫離後，呼叫 _cancel_leader_tasks 取消 keepalive / watch（跳過 current_task 避免自取消 race），清空旗標 進入下一輪選舉。_on_stop 同時 set 該 event 以解封任何 wait。 測試 helper _make_watch_queue 一併移除未使用的 hang_forever 參數， 避免誤導維護端期待。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
問題
Closed-loop probe 發現：
LeaderElector._watch_leader_key（election.py:247）body 是pass空 stub，但 spec 明寫「如果 key 被刪除（非我們自己 resign），觸發 demotion」。實務上 etcd operator 手動etcdctl del、ACL 變更、或 etcd partition 把 key 判給對方，現在的 elector 完全偵測不到 — 必須等 keepalive 連續max_keepalive_failures次失敗（預設約 10s）才轉 demoted，期間 leader 仍認為自己是 leader → 雙寫風險視窗放大。改動
實作 watch loop：
_resigning旗標區分 self-induced vs external DELETE，避免自身 resign 二次觸發on_demoted_handle_demotion()，bounded latency_stop_event協作取消_is_delete_event兼容 etcetraWatchEvent.event: WatchEventType與 mock 的.event_type: str驗證
tests/cluster/test_election_watch.py4 案（external DELETE → demote in bounded time / self resign 不重複 demote / PUT 不誤觸 / watch 異常不 crash）。Pre-fix 4 FAIL → post-fix 全 PASS。_make_mock_client：加 defaultclient.watch回 hang-until-cancelled async generator，避免既有 leader-elected 測試 stop() 卡死。