Skip to content

endpoints to get and delete minion task metadata #8551

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.

Sign up for GitHub

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

Jump to bottom
Merged
Jackie-Jiang merged 2 commits into from
Apr 19, 2022
Merged

endpoints to get and delete minion task metadata #8551

Jackie-Jiang merged 2 commits into from
Apr 19, 2022

Conversation

@klsince
Copy link
Contributor

@klsince klsince commented Apr 15, 2022 •
edited
Loading

add two minion tasks related endpoints to allow GET and DELETE /tasks/{taskType}/{tableNameWithType}/metadata

Release Notes

Added 2 http endpoints for getting and deleting the minion task metadata, like the watermark of RealtimeToOfflineTask.

  • GET /tasks/{taskType}/{tableNameWithType}/metadata
  • DELETE /tasks/{taskType}/{tableNameWithType}/metadata

@klsince klsince force-pushed the endpoints_on_task_metadata branch from 5645ce9 to 8ca9729 Compare April 15, 2022 18:19
@klsince klsince force-pushed the endpoints_on_task_metadata branch from 8ca9729 to 89bf429 Compare April 15, 2022 20:48
@codecov-commenter
Copy link

codecov-commenter commented Apr 15, 2022 •
edited
Loading

Codecov Report

Merging #8551 (138059e) into master (05a7f28) will decrease coverage by 45.06%.
The diff coverage is 27.29%.

❗ Current head 138059e differs from pull request most recent head 0d6edf1. Consider uploading reports for the commit 0d6edf1 to get more accurate results

@@              Coverage Diff              @@
##             master    #8551       +/-   ##
=============================================
- Coverage     70.77%   25.71%   -45.07%     
=============================================
  Files          1681     1675        -6     
  Lines         87957    87894       -63     
  Branches      13320    13314        -6     
=============================================
- Hits          62252    22599    -39653     
- Misses        21354    63135    +41781     
+ Partials       4351     2160     -2191
Flag Coverage Δ
integration1 ?
integration2 25.71% <27.29%> (-0.20%) ⬇️
unittests1 ?
unittests2 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...or/BaseExponentialBackoffRetryFailureDetector.java 0.00% <0.00%> (ø)
...ker/failuredetector/ConnectionFailureDetector.java 0.00% <0.00%> (ø)
...roker/requesthandler/GrpcBrokerRequestHandler.java 78.37% <ø> (ø)
...ker/routing/instanceselector/InstanceSelector.java 100.00% <ø> (ø)
...n/function/scalar/DataTypeConversionFunctions.java 0.00% <0.00%> (ø)
...troller/api/exception/NoTaskMetadataException.java 0.00% <0.00%> (ø)
...roller/api/resources/PinotTaskRestletResource.java 0.00% <0.00%> (-2.92%) ⬇️
...a/org/apache/pinot/core/transport/QueryRouter.java 63.85% <0.00%> (-21.86%) ⬇️
...rg/apache/pinot/core/transport/ServerResponse.java 92.30% <ø> (-3.85%) ⬇️
...va/org/apache/pinot/spi/utils/BigDecimalUtils.java 0.00% <0.00%> (-100.00%) ⬇️
... and 1261 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05a7f28...0d6edf1. Read the comment docs.

mcvsubbu
mcvsubbu reviewed Apr 15, 2022
View reviewed changes
...roller/src/main/java/org/apache/pinot/controller/api/resources/PinotTaskRestletResource.java
return _pinotHelixTaskResourceManager.getTaskMetadataByTable(taskType, tableNameWithType);
}

@DELETE
Copy link
Contributor

@mcvsubbu mcvsubbu Apr 15, 2022

Choose a reason for hiding this comment

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

helix deletes task metadata after sometime, right ? Why provide an endpoint to delete it? Have you verified with helix that this ok? and there is no race condition?

Copy link
Contributor Author

@klsince klsince Apr 16, 2022

Choose a reason for hiding this comment

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

Helix task framework is not aware of the task metadata here, e.g. RealtimToOfflineTask keeps a timestamp as watermark in a ZNode under /Propertystore/MINION_TASK_METADATA/ to manage its progress.

@klsince klsince force-pushed the endpoints_on_task_metadata branch from 0d6edf1 to dd2d22c Compare April 19, 2022 00:52
@klsince klsince force-pushed the endpoints_on_task_metadata branch from dd2d22c to 8d43d8b Compare April 19, 2022 04:34
@Jackie-Jiang Jackie-Jiang merged commit bc363d8 into apache:master Apr 19, 2022
@klsince klsince deleted the endpoints_on_task_metadata branch April 19, 2022 17:05
@Jackie-Jiang Jackie-Jiang added the release-notes Referenced by PRs that need attention when compiling the next release notes label Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcvsubbu mcvsubbu mcvsubbu left review comments

@xiangfu0 xiangfu0 xiangfu0 approved these changes

Assignees

No one assigned

Labels

release-notes Referenced by PRs that need attention when compiling the next release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@klsince @codecov-commenter @xiangfu0 @mcvsubbu @Jackie-Jiang