-
Notifications
You must be signed in to change notification settings - Fork 172
feat(amber): add cache-reused operator state and cached-region statistics handling #6729
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -103,6 +103,7 @@ object Utils extends LazyLogging { | |
| case WorkflowAggregatedState.FAILED => "Failed" | ||
| case WorkflowAggregatedState.KILLED => "Killed" | ||
| case WorkflowAggregatedState.UNKNOWN => "Unknown" | ||
| case WorkflowAggregatedState.CACHE_REUSED => "CacheReused" | ||
| case WorkflowAggregatedState.Unrecognized(unrecognizedValue) => | ||
| s"Unrecognized($unrecognizedValue)" | ||
| } | ||
|
|
@@ -122,6 +123,7 @@ object Utils extends LazyLogging { | |
| case "killed" => WorkflowAggregatedState.KILLED | ||
| case "terminated" => WorkflowAggregatedState.TERMINATED | ||
| case "unknown" => WorkflowAggregatedState.UNKNOWN | ||
| case "cachereused" => WorkflowAggregatedState.CACHE_REUSED | ||
| case other => throw new IllegalArgumentException(s"Unrecognized state: $other") | ||
| } | ||
| } | ||
|
|
@@ -141,6 +143,7 @@ object Utils extends LazyLogging { | |
| case WorkflowAggregatedState.COMPLETED => 3 | ||
| case WorkflowAggregatedState.FAILED => 4 | ||
| case WorkflowAggregatedState.KILLED => 5 | ||
| case WorkflowAggregatedState.CACHE_REUSED => 6 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New status code 6 is not mirrored to the frontend in
Please add the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, thanks. Mirrored code 6 in the frontend: added |
||
| case other => -1 | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you help confirm the intended behavior for mixed cached states? My reading of #5883 is that CACHE_REUSED should behave as a terminal state. With the current fallback, combinations such as COMPLETED + CACHE_REUSED appear to become UNKNOWN, and cached plus paused/ready may also fall through to UNKNOWN. Would it make sense to preserve the all-cached CACHE_REUSED case, then exclude completed, terminated, and cached states before classifying the remainder? This may also matter when resolving the conflict with #6011.