kittimzhe commented on issue #18698: URL: https://github.com/apache/dolphinscheduler/issues/18698#issuecomment-6211632777
Opening the consensus question @SbloodyS asked for before #18709 can come back. **Where we agree** (per your source evidence): crash recovery replays events — `WorkflowFailoverCommandHandler` rebuilds the graph from existing task instances, so a master dying between "persist task failure" and "advance the workflow" does not corrupt the workflow side. #18709 does not dispute that, and never needed to. **The residue the PR targets is narrower**: rows in `t_ds_serial_command` whose instance is terminal, missing, or has a null id. Normal cleanup is same-transaction with workflow finish, and the trigger path persists state before inserting the row in the same transaction — so a stale row is not produced by the crash-between-two-writes window. What remains is defense-in-depth for rows that are already stale when read (upgrades from older deployments, rows orphaned by operational restores, or paths neither of us has enumerated). **The consensus question is about shape, not necessity of the fix.** Three candidates: 1. **Read-time reconciliation** (what #18709 implements): the serial coordinator's poll deletes rows whose instance is terminal/missing/null-id, and drops them from the in-memory list in the same pass. Cost: one extra query per poll cycle. Benefit: self-healing with no operator action. 2. **Acquire-time guard**: check the instance state only when a serial group is about to hand a row to a handler. Cheaper, but a stale row still blocks nothing and lingers in the table. 3. **No runtime change**: treat the table as operational residue, document it, and let a cleanup job or doctor command handle it. My preference is 1, with the null-id and missing-instance cases only (terminal-state deletion is the part that could disagree with a legitimate pause/stop race, and #18709's tests pin that distinction). But if maintainers see the table as strictly derived-and-rebuildable, 3 is defensible and #18709 stays closed. Which shape do we want? Once that's settled, #18709 (or its replacement) comes back matching it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
