-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Add DatafeedTimingStats.average_search_time_per_bucket_ms and TimingStats.total_bucket_processing_time_ms stats #44125
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
Add DatafeedTimingStats.average_search_time_per_bucket_ms and TimingStats.total_bucket_processing_time_ms stats #44125
Conversation
Pinging @elastic/ml-core |
96e4f2a
to
7335f8f
Compare
581dc3b
to
9ffd0dc
Compare
run elasticsearch-ci/packaging-sample |
run elasticsearch-ci/packaging-sample |
1 similar comment
run elasticsearch-ci/packaging-sample |
jobId, | ||
getOrDefault(bucketCount, 0L), | ||
getOrDefault(totalBucketProcessingTimeMs, 0.0), | ||
minBucketProcessingTimeMs, |
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.
I am not sure why we are NOT allowing bucketCount
and totalBucketProcessingTimeMs
to be null but we ARE allowing all the other stats to be null. It seems to be that we should do logical zero values across the board.
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.
My thinking was that we should provide non-null values wherever possible to simplify dealing with the resulting stats.
We can do it for bucketCount and totalBucketProcessingTimeMs because "0" is natural default value for them.
However, Min, Max and AVG need at least one data point to be calculated so there is no natural default value here.
WDYT?
...ore/src/main/java/org/elasticsearch/xpack/core/ml/job/persistence/ElasticsearchMappings.java
Outdated
Show resolved
Hide resolved
.../plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java
Outdated
Show resolved
Hide resolved
...lugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetDatafeedsStatsAction.java
Outdated
Show resolved
Hide resolved
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetJobsStatsAction.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/elasticsearch/xpack/core/ml/job/process/autodetect/state/TimingStats.java
Outdated
Show resolved
Hide resolved
f6fedac
to
1355f4f
Compare
…tats.total_bucket_processing_time_ms stats (elastic#44125)
…e_ms to ml stats Relates: elastic/elasticsearch#44125 This commit adds DatafeedTimingStats to DatafeedStats, and ExponentialAverageBucketProcessingTimePerHourMilliseconds to JobStats.
…e_ms to ml stats (#4163) Relates: elastic/elasticsearch#44125 This commit adds DatafeedTimingStats to DatafeedStats, and ExponentialAverageBucketProcessingTimePerHourMilliseconds to JobStats.
…e_ms to ml stats (#4163) Relates: elastic/elasticsearch#44125 This commit adds DatafeedTimingStats to DatafeedStats, and ExponentialAverageBucketProcessingTimePerHourMilliseconds to JobStats. (cherry picked from commit 83bf3e3)
This PR adds three new fields:
Fields {average_search_time_per_bucket_ms, total_bucket_processing_time_ms} are calculated from other fields and not persisted in the job results index
Related to #29857