-
Notifications
You must be signed in to change notification settings - Fork 94
Return lock information from List Jobs API to see if Job has active/released lock #799
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
Signed-off-by: Jeremy Dupras <[email protected]>
Signed-off-by: Jeremy Dupras <[email protected]>
Codecov ReportAttention: Patch coverage is
❌ Your project status has failed because the head coverage (32.56%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #799 +/- ##
============================================
- Coverage 33.54% 32.56% -0.99%
Complexity 143 143
============================================
Files 29 29
Lines 1389 1431 +42
Branches 132 135 +3
============================================
Hits 466 466
- Misses 886 928 +42
Partials 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I see an IndexOutOfBoundsException in the multi-node test check:
|
return new GetScheduledInfoNodeResponse(in); | ||
} | ||
|
||
private List<Map<String, Object>> findLockByJobId(String jobId) { |
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.
Let's inject the LockService into the constructor and then call findLock
like this: https://github.com/opensearch-project/job-scheduler/blob/main/spi/src/main/java/org/opensearch/jobscheduler/spi/utils/LockService.java#L157
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.
FYI ActionListener is similar to a Promise in JavaScript where you have to define the success and failure case.
In this method actionGet
behaves similar to await
in JavaScript where it blocks the thread and awaits results of the async call. We should use async calls here to not be blocking.
Signed-off-by: Jeremy Dupras <[email protected]>
Signed-off-by: Jeremy Dupras <[email protected]>
Signed-off-by: Jeremy Dupras <[email protected]>
Signed-off-by: Jeremy Dupras <[email protected]>
Signed-off-by: Jeremy Dupras <[email protected]>
Signed-off-by: Jeremy Dupras <[email protected]>
Signed-off-by: Jeremy Dupras <[email protected]>
Signed-off-by: Jeremy Dupras <[email protected]>
Signed-off-by: Jeremy Dupras <[email protected]>
// Add lock information | ||
// jobDetails.put("lock", findLockByJobId(jobId)); | ||
|
||
CountDownLatch latch = new CountDownLatch(1); |
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 think it makes sense to have 2 separate APIs here to avoid the blocking operations like actionGet
and CountDownLatch
.
wdyt about separating out list lock into a separate API? In the API handler for listing the locks, let's use the scroll API to get all records in the index.
Description
Adds lock information to the Existing Job information API. Lock information includes. Index name, lock time, job ID, lock duration, and released status. See an example output below.
Related Issues
This will be used as a method to get the currently executing jobs.
Check List
--signoff
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.