Two fixes to work coordination #777
Conversation
…ore reliable and only scan for work that is currently expired. The HEAD check has now been rolled into the doUntil block for running setup() so that even 400s from the PUT command can be ignored and retried again in a sane manner. The HEAD call should be the authoritative call when it returns 200. The second fix is for searching for items to update. I had been searching for items that weren't even updated yet, causing a lot more pressure to create false positives and inefficiencies all around. Signed-off-by: Greg Schohn <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #777 +/- ##
============================================
+ Coverage 68.53% 69.21% +0.68%
- Complexity 1577 1594 +17
============================================
Files 275 275
Lines 11660 11660
Branches 737 737
============================================
+ Hits 7991 8071 +80
+ Misses 3263 3168 -95
- Partials 406 421 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
gregschohn
changed the title
gregschohn
requested review from
AndreKurait,
chelma,
lewijacn,
mikaylathompson,
okhasawn,
peternied and
sumobrian
as code owners
| @@ -123,7 +123,7 @@ public String toString() { | |||
| return "[ statusCode: " + r.getStatusCode() + ", payload: " + payloadStr + "]"; | |||
| } | |||
| }, | |||
| (response, ignored) -> (response.getStatusCode() / 100) != 2); | |||
There was a problem hiding this comment.
Just curious - what was the original logic you were trying to achieve here? I didn't quite follow your explanation in the PR overview.
There was a problem hiding this comment.
The test determines when we're done needing to retry. When this is 'true', the work is deemed to be done. In other words, we would keep looping until we got something other than a 200. Notice that the beginning of the method was a HEAD that didn't have the same affliction - so in the worst case scenario, we'd burn a process and be resolved the next time that the program ran
Description
Make the check for the setup index more reliable and only scan for work that is currently expired.
The HEAD check has now been rolled into the doUntil block for running setup() so that even 400s from the PUT command can be ignored and retried again in a sane manner. The HEAD call should be the authoritative call when it returns 200. The second fix is for searching for items to update. I had been searching for items that weren't even updated yet, causing a lot more pressure to create false positives and inefficiencies all around.
Issues Resolved
https://opensearch.atlassian.net/browse/MIGRATIONS-1810
Testing
Manual/gradle build
Check List
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.