Skip to content
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 metrics for batch pending max elapse time #6829

Closed
wants to merge 7 commits into from

Conversation

turboFei
Copy link
Member

@turboFei turboFei commented Dec 1, 2024

Why are the changes needed?

  1. add metrics kyuubi.operartion.batch_pending_max_elapse for the batch pending max elapse time, which is helpful for batch health monitoring, and we can send alert if the batch pending elapse time too long
  2. For GET /api/v1/batches api, limit the max time window for listing batches, which is helpful that, we want to reserve more metadata in kyuubi server end, for example: 90 days, but for list batches, we just want to allow user to search the last 7 days. It is optional. And if create_time is specified, order by create_time instead of key_id.

How was this patch tested?

GA.

Was this patch authored or co-authored using generative AI tooling?

No.

@turboFei turboFei changed the title Batch pending time Add metrics for batch pending max elapse time Dec 1, 2024
metrics

pending

doc

limit

fix
@codecov-commenter
Copy link

codecov-commenter commented Dec 1, 2024

Codecov Report

Attention: Patch coverage is 0% with 29 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (2ab2789) to head (ee4f931).
Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
...ache/kyuubi/server/KyuubiRestFrontendService.scala 0.00% 6 Missing ⚠️
...pache/kyuubi/server/metadata/MetadataManager.scala 0.00% 6 Missing ⚠️
...in/scala/org/apache/kyuubi/config/KyuubiConf.scala 0.00% 5 Missing ⚠️
...g/apache/kyuubi/operation/BatchJobSubmission.scala 0.00% 3 Missing ⚠️
.../apache/kyuubi/server/api/v1/BatchesResource.scala 0.00% 3 Missing ⚠️
...yuubi/server/metadata/jdbc/JDBCMetadataStore.scala 0.00% 3 Missing ⚠️
.../apache/kyuubi/server/metadata/MetadataStore.scala 0.00% 2 Missing ⚠️
...a/org/apache/kyuubi/metrics/MetricsConstants.scala 0.00% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master   #6829   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         687     687           
  Lines       42439   42463   +24     
  Branches     5791    5796    +5     
======================================
- Misses      42439   42463   +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@turboFei turboFei requested a review from pan3793 December 1, 2024 23:03
@turboFei turboFei self-assigned this Dec 2, 2024
@turboFei turboFei added this to the v1.10.1 milestone Dec 2, 2024
val filter = MetadataFilter(
sessionType = SessionType.BATCH,
engineType = batchType,
username = batchUser,
state = batchState,
requestName = batchName,
createTime = createTime,
createTime = createTimeFilter,
endTime = endTime)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not related to this PR, but createTime and endTime are confusing, should be minCreateTime and maxEndTime, and say if they are inclusive or exclusive in the comments, with an explanation for specific values

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update: the predication name might be more intuitive if we use greatThanXXX lessThanOrEqualsXXX

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will address it in another PR

@turboFei
Copy link
Member Author

turboFei commented Dec 2, 2024

thanks for the comments, will address it today

@turboFei turboFei force-pushed the batch_pending_time branch 2 times, most recently from e8aa576 to d9bc335 Compare December 2, 2024 22:22
@turboFei turboFei requested a review from pan3793 December 3, 2024 04:45
@turboFei turboFei requested a review from pan3793 December 3, 2024 17:40
@pan3793 pan3793 closed this in 3167692 Dec 5, 2024
pan3793 pushed a commit that referenced this pull request Dec 5, 2024
### Why are the changes needed?

1. add metrics `kyuubi.operartion.batch_pending_max_elapse` for the batch pending max elapse time, which is helpful for batch health monitoring, and we can send alert if the batch pending elapse time too long
2. For `GET /api/v1/batches` api, limit the max time window for listing batches, which is helpful that, we want to reserve more metadata in kyuubi server end, for example: 90 days, but for list batches, we just want to allow user to search the last 7 days. It is optional. And if `create_time` is specified, order by `create_time` instead of `key_id`.
https://github.com/apache/kyuubi/blob/68a6f48da53dd0ad2e20b450a41ca600b8c1e1d2/kyuubi-server/src/main/resources/sql/mysql/metadata-store-schema-1.8.0.mysql.sql#L32

### How was this patch tested?

GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #6829 from turboFei/batch_pending_time.

Closes #6829

ee4f931 [Wang, Fei] docs
bf8169a [Wang, Fei] comments
f493a2a [Wang, Fei] new config
ab7b6db [Wang, Fei] ut
1680175 [Wang, Fei] in memory session
510a30b [Wang, Fei] batchSearchWindow opt
1e93dd2 [Wang, Fei] save

Authored-by: Wang, Fei <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
(cherry picked from commit 3167692)
Signed-off-by: Cheng Pan <[email protected]>
@pan3793
Copy link
Member

pan3793 commented Dec 5, 2024

Thanks, merged to master/1.10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants