-
Notifications
You must be signed in to change notification settings - Fork 5.5k
fix: Add createTime and endTime to task RuntimeStats to match native #26669
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
Conversation
Reviewer's GuideThe PR introduces createTime and endTime metrics into TaskContext’s RuntimeStats to match native behavior by capturing and injecting these timestamps into mergedRuntimeStats and updating TaskStats construction, along with adding end-to-end tests verifying the new metrics before and after task completion. Entity relationship diagram for updated RuntimeStats metricserDiagram
TASK_CONTEXT ||--o{ RUNTIME_STATS : updates
RUNTIME_STATS {
string metricName
string metricType
long metricValue
}
RUNTIME_STATS ||--|{ TASK_STATS : included_in
TASK_STATS {
long createTimeInMillis
long endTimeInMillis
}
Class diagram for updated TaskContext and TaskStatsclassDiagram
class TaskContext {
+getTaskStats()
-taskStateMachine
-executionEndTime
-mergedRuntimeStats
}
class TaskStats {
+createTimeInMillis: long
+endTimeInMillis: long
+executionStartTime
+lastExecutionStartTime
+lastExecutionEndTime
+elapsedTimeInNanos
+queuedTimeInNanos
+totalDrivers
}
class RuntimeStats {
+addMetricValue(name: String, type, value: long)
}
TaskContext --> TaskStats
TaskContext --> RuntimeStats
TaskStats o-- "createTimeInMillis" long
TaskStats o-- "endTimeInMillis" long
RuntimeStats <.. TaskContext : "addMetricValue('createTime', ...)"
RuntimeStats <.. TaskContext : "addMetricValue('endTime', ...)"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
f9be4f2 to
45c994f
Compare
tanjialiang
left a comment
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.
Thanks LGTM
45c994f to
aa8c585
Compare
NikhilCollooru
left a comment
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.
Lgtm
Presto on Spark (and Presto Java) are missing the logging of Task createTime and endTime RuntimeMetric. The Presto Native Task execution logs these.
These metrics are used internally at Meta to show stage level start and end time visualizations.
This PR adds them so that we can leverage this internal tooling for Presto-on-Spark Java queries. Specially useful when comparing performance/runtime of java v/s native presto-on-spark queries
For reference, internally at Meta, adding these metrics enables this visualization
