Keep a finished workflow's node statuses current instead of expiring them - #1298
Open
romer8 wants to merge 2 commits into
Open
Keep a finished workflow's node statuses current instead of expiring them#1298romer8 wants to merge 2 commits into
romer8 wants to merge 2 commits into
Conversation
…them Persisted node statuses are served only while they are recent, so that a deployment whose reporter has stopped falls back to reading live statuses rather than showing values that never advance. That rule is right while a workflow is running and pointless once it has finished: a workflow that has left the queue with every node in a status it cannot leave will never report again, so expiring its statuses only sends the views to the scheduler to be told the same thing. It is also the case where reading live statuses is most likely to fail. Working directories of finished workflows are eventually cleaned up, and condorpy chdirs into one to query the queue, so the fallback raises FileNotFoundError and the whole details row fails to render. Both halves of the new condition are load-bearing. The workflow's own status is not enough, because a node can be reported terminal and then requeued -- a held node that DAGMan retries -- and the workflow is only terminal once DAGMan has departed. The nodes are not enough either: a reporter that never sends a final report leaves the last node non-terminal, and that copy must not be trusted forever. Requiring both means a workflow recorded by such a reporter keeps expiring and reading live, which is the behaviour it has today. A node with no persisted status counts against the set rather than being skipped, so a workflow nothing has ever reported on is unaffected.
|
coverage: 99.993% (-0.007%) from 100.0% — Aquaveo:fix/trust-terminal-node-statuses into tethysplatform:main |
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.
Description
Persisted node statuses expire after 60s so a stopped reporter falls back to live statuses. That is right while a workflow runs and pointless once it finishes — a workflow that left the queue with every node terminal will never report again. It is also where the fallback most often fails: finished workflows' working directories get cleaned up, and condorpy
chdirs into one, so the details row raisesFileNotFoundError.Requires both the workflow and every node to be terminal. The workflow alone is not enough (a held node can be requeued); the nodes alone are not enough (a reporter that never sends a final report leaves the last node non-terminal, and that copy must keep expiring).
Changes Made to Code
node_statuses_are_currentreturns true for a finished workflow whose nodes are all terminal.all_node_statuses_are_terminal; a node with no status counts against the set.