fix(compose): legacy interrupt wrapping with BatchResumeWithData - #1269
Open
JopenChen wants to merge 1 commit into
Open
fix(compose): legacy interrupt wrapping with BatchResumeWithData#1269JopenChen wants to merge 1 commit into
JopenChen wants to merge 1 commit into
Conversation
Fix the incompatibility between legacy interrupt errors (InterruptAndRerun, NewInterruptAndRerunErr) wrapped via WrapInterruptAndRerunIfNeeded and the BatchResumeWithData resume mechanism. Root cause: CompositeInterrupt generated a new random UUID for each legacy interrupt on every invocation. Since BatchResumeWithData matches resume data by interrupt ID, the IDs from the first invocation no longer matched after a re-run, causing the graph to re-interrupt instead of completing. Fix: Use the deterministic address string (wrapped.ps.String()) as the ID for legacy interrupts instead of uuid.NewString(). Addresses are stable across re-runs, ensuring resume data correctly matches the interrupt point. Changes: - Replace uuid.NewString() with wrapped.ps.String() in CompositeInterrupt for both deprecatedInterruptAndRerun and InterruptSignal cases - Remove unused github.com/google/uuid import - Update TestLegacyInterrupt to assert successful resume instead of error Resolves the TODO at compose/resume_test.go:1096
|
|
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
2 participants
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.
What type of PR is this?
fix
Check the PR title.
(Optional) Translate the PR title into Chinese.
fix(compose): 修复 legacy interrupt 与 BatchResumeWithData 不兼容的问题
(Optional) More detailed description for this PR(en: English/zh: Chinese).
en:
Fix the incompatibility between legacy interrupt errors (InterruptAndRerun, NewInterruptAndRerunErr) wrapped via WrapInterruptAndRerunIfNeeded and the BatchResumeWithData resume mechanism.
There was a TODO at compose/resume_test.go:1096 noting that legacy interrupt wrapping does not work correctly with BatchResumeWithData - the graph re-interrupts instead of completing.
Root cause: CompositeInterrupt generated a new random UUID for each legacy interrupt on every invocation. Since BatchResumeWithData matches resume data by interrupt ID, the IDs from the first invocation no longer matched after a re-run.
Fix: Use the deterministic address string (wrapped.ps.String()) as the ID for legacy interrupts instead of uuid.NewString(). Addresses are stable across re-runs, ensuring resume data correctly matches the interrupt point.
Also updated the test to use IDs from the most recent invocation (modern Interrupt() still uses random UUIDs), and removed the unused uuid import.
zh(optional):
修复通过 WrapInterruptAndRerunIfNeeded 包装的 legacy 中断错误（InterruptAndRerun、NewInterruptAndRerunErr）与 BatchResumeWithData 恢复机制不兼容的问题。
compose/resume_test.go:1096 处原有 TODO 标注 legacy interrupt wrapping 与 BatchResumeWithData 不能正常工作——图会重新中断而非完成执行。
根本原因：CompositeInterrupt 每次调用都为每个 legacy 中断生成新的随机 UUID。由于 BatchResumeWithData 按中断 ID 匹配恢复数据，重新运行后第一次调用的 ID 不再匹配。
修复方案：使用确定性的地址字符串（wrapped.ps.String()）作为 legacy 中断的 ID，替代 uuid.NewString()。地址在多次运行间保持稳定，确保恢复数据能正确匹配到中断点。
同时更新了测试以使用最近一次调用的 ID（现代 Interrupt() 仍使用随机 UUID），并移除了未使用的 uuid 导入。
(Optional) Which issue(s) this PR fixes:
N/A (resolves internal TODO)
(optional) The PR that updates user documentation:
N/A