Allow user to provide batch id on submitting batch job #4390

pan3793 · 2023-02-21T06:17:51Z

Why are the changes needed?

This PR proposes to allow the user to provide a batch id on submitting a batch job. If the batch id already existed in metastore, Kyuubi ignores this submission and just returns the existing one, w/ a marker in response, this could avoid duplicated batch job submission.

This feature relies on #4418

Talking about the implementation, the key things are

How does the user set the custom batch id?

User can optionally set the kyuubi.batch.id in conf: Map[String, String], and the value must be a UUID, for Java users, it can be generated by UUID.randomUUID().toString()

How does the Kyuubi Server detect the duplication?

It's simple in single Kyuubi Server instance case, Kyuubi just needs to look up the metastore before creating a batch job
In HA mode, suppose the user requests to create the batch jobs w/ the same batch id concurrently, multiple Kyuubi Servers may process the request and try to insert to metastore DB at the same time, but only the first insertion success, others will fail w/ "duplicated key", Kyuubi Server needs to catch this error and return the existing batch job information instead of creating a new one.

How does the user know if the returned batch job is new created or duplicated?

a new field batchInfo: Map[String, String] is added to the response, and for duplicated batch job, "kyuubi.batch.duplicated": "true" will be contained.

How was this patch tested?

Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala

turboFei

left some comments

turboFei · 2023-02-21T06:48:48Z

seems mistake, let me think it twice.

ulysses-you · 2023-02-21T07:32:10Z

How about add a traceToken in rest client ? Trino support that and Kyuubi server should return traceToken back to client. Then developer can deduplicate by itself.

pan3793 · 2023-02-21T07:49:39Z

How about add a traceToken in rest client ? Trino support that and Kyuubi server should return traceToken back to client. Then developer can deduplicate by itself.

I think it makes things more complex, IMO let the user provide a custom batch id does not has many risks, we already restrict the batch id MUST be a UUID.

pan3793 · 2023-03-01T04:20:08Z

cc @turboFei @ulysses-you it's ready for the next review.

codecov-commenter · 2023-03-02T05:28:43Z

Codecov Report

Merging #4390 (b6917ba) into master (abc0a1d) will decrease coverage by 0.01%.
The diff coverage is 81.53%.

@@             Coverage Diff              @@
##             master    #4390      +/-   ##
============================================
- Coverage     53.24%   53.24%   -0.01%     
  Complexity       13       13              
============================================
  Files           569      569              
  Lines         31111    31146      +35     
  Branches       4203     4208       +5     
============================================
+ Hits          16565    16583      +18     
- Misses        12979    12992      +13     
- Partials       1567     1571       +4

Impacted Files	Coverage Δ
...ava/org/apache/kyuubi/client/api/v1/dto/Batch.java	`88.23% <75.00%> (-1.93%)`	⬇️
.../apache/kyuubi/server/api/v1/BatchesResource.scala	`70.58% <75.60%> (-1.04%)`	⬇️
.../main/scala/org/apache/kyuubi/util/JdbcUtils.scala	`89.74% <100.00%> (+1.17%)`	⬆️
.../apache/kyuubi/client/api/v1/dto/BatchRequest.java	`56.86% <100.00%> (+3.92%)`	⬆️
...java/org/apache/kyuubi/client/util/BatchUtils.java	`80.00% <100.00%> (+5.00%)`	⬆️
...pache/kyuubi/server/metadata/MetadataManager.scala	`80.72% <100.00%> (-0.23%)`	⬇️
...apache/kyuubi/session/KyuubiBatchSessionImpl.scala	`86.81% <100.00%> (ø)`
...e/kyuubi/jdbc/hive/ClosedOrCancelledException.java	`0.00% <0.00%> (-100.00%)`	⬇️
...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala	`59.09% <0.00%> (-7.58%)`	⬇️
...in/spark/authz/ranger/SparkRangerAdminPlugin.scala	`64.47% <0.00%> (-2.64%)`	⬇️
... and 3 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

turboFei · 2023-03-02T09:35:52Z

TODO: render the batchInfo in kyuubi-ctl

pan3793 · 2023-03-02T09:42:18Z

render the batchInfo in kyuubi-ctl

Tracking in #4438

pan3793 · 2023-03-02T09:43:06Z

Thanks, merged to master/1.7

### _Why are the changes needed?_ This PR proposes to allow the user to provide a batch id on submitting a batch job. If the batch id already existed in metastore, Kyuubi ignores this submission and just returns the existing one, w/ a marker in response, this could avoid duplicated batch job submission. Talking about the implementation, the key things are How does the user set the custom batch id? - User can optionally set the `kyuubi.batch.id` in `conf: Map[String, String]`, and the value must be a UUID, for Java users, it can be generated by `UUID.randomUUID().toString()` How does the Kyuubi Server detect the duplication? - It's simple in single Kyuubi Server instance case, Kyuubi just needs to look up the metastore before creating a batch job - In HA mode, suppose the user requests to create the batch jobs w/ the same batch id concurrently, multiple Kyuubi Servers may process the request and try to insert to metastore DB at the same time, but only the first insertion success, others will fail w/ "duplicated key", Kyuubi Server needs to catch this error and return the existing batch job information instead of creating a new one. How does the user know if the returned batch job is new created or duplicated? - a new field `batchInfo: Map[String, String]` is added to the response, and for duplicated batch job, `"kyuubi.batch.duplicated": "true"` will be contained. ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request Closes #4390 from pan3793/batch-id. Closes #4390 b6917ba [Cheng Pan] move constant to rest client 79ef1b5 [Cheng Pan] flaky test f822285 [Cheng Pan] it 88bdfa5 [Cheng Pan] ut fd8bc22 [Cheng Pan] ut c820f5e [Cheng Pan] Support user provided batch id on batch job submission Authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Cheng Pan <chengpan@apache.org> (cherry picked from commit efbaaff) Signed-off-by: Cheng Pan <chengpan@apache.org>

### _Why are the changes needed?_ Close #4438 Follow up of #4390 ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request Closes #4463 from turboFei/render_batch. Closes #4438 b5ae211 [fwang12] render batch info Authored-by: fwang12 <fwang12@ebay.com> Signed-off-by: fwang12 <fwang12@ebay.com>

### _Why are the changes needed?_ Close #4438 Follow up of #4390 ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request Closes #4463 from turboFei/render_batch. Closes #4438 b5ae211 [fwang12] render batch info Authored-by: fwang12 <fwang12@ebay.com> Signed-off-by: fwang12 <fwang12@ebay.com> (cherry picked from commit 39eac5a) Signed-off-by: fwang12 <fwang12@ebay.com>

github-actions bot added module:common module:server labels Feb 21, 2023

pan3793 requested a review from turboFei February 21, 2023 06:18

turboFei reviewed Feb 21, 2023

View reviewed changes

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala Outdated Show resolved Hide resolved

turboFei reviewed Feb 21, 2023

View reviewed changes

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala Show resolved Hide resolved

turboFei requested changes Feb 21, 2023

View reviewed changes

turboFei self-requested a review February 22, 2023 09:26

pan3793 force-pushed the batch-id branch from d8c2d86 to 997d1ae Compare February 26, 2023 09:24

github-actions bot added module:ctl module:rest-client labels Feb 26, 2023

pan3793 marked this pull request as draft February 26, 2023 10:16

pan3793 force-pushed the batch-id branch from 997d1ae to 85e7343 Compare February 26, 2023 21:03

github-actions bot removed the module:ctl label Feb 26, 2023

Support user provided batch id on batch job submission

c820f5e

pan3793 force-pushed the batch-id branch from 85e7343 to c820f5e Compare March 1, 2023 04:02

pan3793 self-assigned this Mar 1, 2023

pan3793 marked this pull request as ready for review March 1, 2023 04:19

pan3793 added 3 commits March 1, 2023 12:59

ut

fd8bc22

ut

88bdfa5

it

f822285

github-actions bot added module:integration-tests module:kubernetes labels Mar 1, 2023

pan3793 added 2 commits March 1, 2023 16:15

flaky test

79ef1b5

move constant to rest client

b6917ba

ulysses-you approved these changes Mar 2, 2023

View reviewed changes

pan3793 added this to the v1.7.0 milestone Mar 2, 2023

turboFei approved these changes Mar 2, 2023

View reviewed changes

pan3793 mentioned this pull request Mar 2, 2023

[Improvement] Render the batchInfo in kyuubi-ctl #4438

Closed

4 tasks

pan3793 closed this in efbaaff Mar 2, 2023

pan3793 deleted the batch-id branch March 3, 2023 03:17

turboFei mentioned this pull request Mar 6, 2023

[KYUUBI #4438] Render the batch info map in kyuubi-ctl #4463

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow user to provide batch id on submitting batch job #4390

Allow user to provide batch id on submitting batch job #4390

Uh oh!

pan3793 commented Feb 21, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

turboFei left a comment

Uh oh!

turboFei commented Feb 21, 2023

Uh oh!

ulysses-you commented Feb 21, 2023

Uh oh!

pan3793 commented Feb 21, 2023

Uh oh!

pan3793 commented Mar 1, 2023

Uh oh!

codecov-commenter commented Mar 2, 2023 •

edited

Loading

Uh oh!

turboFei commented Mar 2, 2023

Uh oh!

pan3793 commented Mar 2, 2023

Uh oh!

pan3793 commented Mar 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Allow user to provide batch id on submitting batch job #4390

Allow user to provide batch id on submitting batch job #4390

Uh oh!

Conversation

pan3793 commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are the changes needed?

How was this patch tested?

Uh oh!

Uh oh!

Uh oh!

turboFei left a comment

Choose a reason for hiding this comment

Uh oh!

turboFei commented Feb 21, 2023

Uh oh!

ulysses-you commented Feb 21, 2023

Uh oh!

pan3793 commented Feb 21, 2023

Uh oh!

pan3793 commented Mar 1, 2023

Uh oh!

codecov-commenter commented Mar 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

turboFei commented Mar 2, 2023

Uh oh!

pan3793 commented Mar 2, 2023

Uh oh!

pan3793 commented Mar 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pan3793 commented Feb 21, 2023 •

edited

Loading

codecov-commenter commented Mar 2, 2023 •

edited

Loading