fix(equipment): ReadScheduler rotating-slot partial-fail 不再 silent skip#145
Open
csp0924 wants to merge 2 commits into
Open
fix(equipment): ReadScheduler rotating-slot partial-fail 不再 silent skip#145csp0924 wants to merge 2 commits into
csp0924 wants to merge 2 commits into
Conversation
…ent skip 問題：ReadScheduler.get_next_groups() 被呼叫時無條件推進 _rotating_index， 而 GroupReader.read_many() 對個別 group 失敗會 swallow（gather return_exceptions=True + log+continue）並回傳 partial dict。 AsyncModbusDevice._read_all 沒偵測這種「rotating slot 完全失敗」情況， 導致該 slot 要等下一整輪 K-cycle 才會被重訪 — silent data staleness。 修法： - ReadScheduler 新增 rollback_index() 與 get_next_groups_with_rotating()， 後者額外回傳本次的 rotating slice，供呼叫端做完整性檢查。 - AsyncModbusDevice._read_all 在 read_many 回傳後比對 rotating slot 預期 的點位名是否齊全；任一缺席就 rollback_index → 下個 cycle 重訪同一 slot。 - 完整性檢查放在 aggregator pipeline 之前，避免 aggregator 改名 / 合併 keys 造成判斷失準。 Sandbox 重跑（K=10/fail=10%）： - 修復前：max success interval ≈ 40 ticks（4.0 × K），多個 slot starved。 - 修復後：max success interval = 16 ticks（1.6 × K），0 starved slots。 K=10/fail=30% 同樣從 70 ticks 降至 24 ticks，intervals > 5K 從多筆變為 0。 公開 API：純新增 method（rollback_index, get_next_groups_with_rotating）， 既有 get_next_groups / peek_next_groups 行為保留 → patch bump。
There was a problem hiding this comment.
Pull request overview
This PR fixes a rotating-slot polling starvation issue in the equipment read loop where per-group failures inside GroupReader.read_many() could be swallowed (partial dict returned), causing the scheduler’s rotating index to advance and the failed slot to be skipped for an entire K-cycle.
Changes:
- Add
ReadScheduler.get_next_groups_with_rotating()andReadScheduler.rollback_index()to support “advance + rollback on failure” semantics. - Update
AsyncModbusDevice._read_all()to detect incomplete rotating-slot results (missing expected point names) and rollback the scheduler index to retry the same slot next cycle. - Add a new test suite covering scheduler rollback behavior and device-level rotating-slot retry/advance sequences.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
csp_lib/equipment/transport/scheduler.py |
Adds APIs to return the rotating slice alongside groups and to rollback the rotating index after an advance. |
csp_lib/equipment/device/base.py |
Adds rotating-slice completeness checks after read_many() and rolls back the rotating index when incomplete to avoid silent skips. |
tests/equipment/test_scheduler_rotating_failure.py |
Adds regression + integration tests for rotating-slot rollback/retry behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+739
to
+740
| 要等一整輪 K-cycle 才會被重訪的 silent staleness（見 | ||
| ``sandbox/read_scheduler_rotating_silent_skip_demo.py``）。 |
Comment on lines
+751
to
+761
| # rotating slot 完整性檢查：任一預期點位缺席就 rollback 讓下輪重訪 | ||
| if rotating_slice: | ||
| expected_names = {p.name for g in rotating_slice for p in g.points} | ||
| if expected_names and not expected_names.issubset(raw_values.keys()): | ||
| missing = expected_names - raw_values.keys() | ||
| logger.warning( | ||
| f"[{self._config.device_id}] Rotating slot read incomplete, " | ||
| f"rolling back rotating_index for retry next cycle. missing={sorted(missing)}" | ||
| ) | ||
| self._scheduler.rollback_index() | ||
|
|
Copilot review #145 反饋： 1. 每次 rotating slot 讀取不完整就 rollback rotating_index，會讓一個永久 失敗的 slot starve 後續其他 slot（永遠讀不到）。新增上限：同一 slot 連續 rollback 達 _MAX_ROTATING_ROLLBACKS (=3) 次後放棄重試，index 正常推進 並紀錄 WARNING，讓 rotating queue 繼續輪轉到其他 slot。 2. 移除 _read_all docstring 中對不存在檔案 sandbox/read_scheduler_rotating _silent_skip_demo.py 的指引。 成功讀取或切換到不同 slot 時自動 reset 計數，避免假性累積。
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.
Bug
AsyncModbusDevice._read_all流程：read_many對 per-group exception 用return_exceptions=Trueswallow + log，回傳 partial dict 不含失敗 group 的 keys。_read_all無 except 不會回滾 rotating index → 失敗的 rotating slot 整輪 K-cycle 不會被重訪。下游 alarms / EMS / Modbus gateway 看到 stale value 跨 ≥ K × read_interval。Closed-loop sandbox 量化
K=10、固定 fail rate：
pre-fix
P[gap > nK] = fail_pct^n（geometric tail，無上界）；post-fix 失敗 slot 下個 cycle 重訪、不再被連續跳。Fix（方案 A：最小侵入 patch-safe）
csp_lib/equipment/transport/scheduler.py純新增：rollback_index()— index 回退一格、無 rotating 時 no-opget_next_groups_with_rotating() -> tuple[list[ReadGroup], list[ReadGroup]]— 同樣 advance index，額外回傳本次 rotating slice 供 caller 完整性檢查csp_lib/equipment/device/base.py::_read_all改用新 method，比對 rotating slot 預期點位齊不齊（在 aggregator pipeline 之前做，避免 key 改名 / 合併干擾），缺就rollback_index()+ log warning。無 public API 變更、無 exception 類型 / return-type 語義變更 → patch 版本。
Tests
tests/equipment/test_scheduler_rotating_failure.py（新檔，11 case）：Scheduler-level：rollback after advance / wrap at 0 / no-rotating no-op / single slot
API：tuple return shape / advance / empty rotating
Device-level integration：full failure rollback / happy path 不破壞 / 失敗→重試→推進序列 / partial in-slot failure
Pre-fix 10 fail / 1 pass → Post-fix 11/11 pass。
Quality gates