Conversation
There was a problem hiding this comment.
I know previous behavior was to spawn a thread every time. I think it will be cleaner to return a boolean from the deletion methods to say whether the index deletion was succesful or is marked as pending and only spawn the thread for the latter.
There was a problem hiding this comment.
Agree this would be a good thing to do. As this touches a lot more places in the code, I will do a follow-up.
|
LGTM, with a minor suggestion. This introduces old behavior and I know that is what you wanted to achieve here. Long term I think we can clean up more things here (like shard level pending deletes which are really only processed as far as I can tell when index deletion happens). |
Triggering the pending deletes logic was accidentally removed in the clean up PR elastic#18602.
7467aac to
736d772
Compare
|
Thanks for the review, @bleskes. I've added a comment to |
* master: (184 commits) Add back pending deletes (#18698) refactor matrix agg documentation from modules to main agg section Implement ctx.op = "delete" on _update_by_query and _reindex Close SearchContext if query rewrite failed Wrap lines at 140 characters (:qa projects) Remove log file painless: Add support for the new Java 9 MethodHandles#arrayLength() factory (see https://bugs.openjdk.java.net/browse/JDK-8156915) More complete exception message in settings tests Use java from path if JAVA_HOME is not set Fix uncaught checked exception in AzureTestUtils [TEST] wait for yellow after setup doc tests (#18726) Fix recovery throttling to properly handle relocating non-primary shards (#18701) Fix merge stats rendering in RestIndicesAction (#18720) [TEST] mute RandomAllocationDeciderTests.testRandomDecisions Reworked docs for index-shrink API (#18705) Improve painless compile-time exceptions Adds UUIDs to snapshots Add test rethrottle test case for delete-by-query Do not start scheduled pings until transport start Adressing review comments ...
It was lost in #18602