[Test] More reliable wait for index to appear #126437
Conversation
Relates: elastic#125652 Resolves: elastic#126204
ywangd
added
>test
elasticsearchmachine
added
the
Team:Distributed Coordination
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
| @@ -748,7 +748,7 @@ public void testDeletionOfFailingToRecoverIndexShouldStopRestore() throws Except | |||
|
|
|||
| logger.info("--> wait for the index to appear"); | |||
| // that would mean that recovery process started and failing | |||
| waitForIndex("test-idx", TimeValue.timeValueSeconds(10)); | |||
There was a problem hiding this comment.
This is the only caller of waitForIndex so I suggest removing the method itself too.
| @@ -748,7 +748,7 @@ public void testDeletionOfFailingToRecoverIndexShouldStopRestore() throws Except | |||
|
|
|||
| logger.info("--> wait for the index to appear"); | |||
| // that would mean that recovery process started and failing | |||
| waitForIndex("test-idx", TimeValue.timeValueSeconds(10)); | |||
| safeGet(clusterAdmin().prepareHealth(TEST_REQUEST_TIMEOUT, "test-idx").execute()); | |||
There was a problem hiding this comment.
No need to set a 30s timeout on the request if we're only waiting for 10s:
| safeGet(clusterAdmin().prepareHealth(TEST_REQUEST_TIMEOUT, "test-idx").execute()); | |
| safeGet(clusterAdmin().prepareHealth(SAFE_AWAIT_TIMEOUT, "test-idx").execute()); |
ywangd
added
the
auto-merge-without-approval
|
As far as I am concerned, the cluster health API is also a candidate for being run on the local node by default - it's also listed in #101805. I still think that's valuable and worth doing, but that API will probably need the option to run on the master node, as we'll want to be able to wait for all tasks to complete on the master node - at least in tests, not sure how valuable that is in production. This is not something we need to discuss on this PR - I only brought it up here because I noticed we're using the cluster health API here to check the index health - but we might need to discuss an approach sometime later. |
|
++ yeah the cluster health API is kind of a weird one, it's so heavily used in tests to wait for the master node state that I think making it node-local will be fraught with difficulty. We could for instance wait for the node-local state and then reach out to the master to wait for the application to complete? But also it's not particularly heavyweight, it might not be worth the effort to migrate it. |
Relates: #125652
Resolves: #126204