Fix Watcher ExecuteWatchQueuedStatsTests

[jakelandis]

Attempt to stabilize ExecuteWatchQueuedStatsTests.
This test is tricky since it non-reproducible old style integration
test that runs 3 nodes with Watcher and digs in pretty deep into
the implementation details. It is believed that there is a race condition
between the testing concurrency and Watcher concurrency such
that Watcher will schedule a new Watch at just the wrong time such that
a future is created but Watcher gets shut down before it complete
which results in the test waiting indefinitely for a future that can never return.

The main fix here is to bump the interval up from 1s to 1h such that Watcher
will not schedule second watch that will never finish. There secondary fixes
are to place an upper time bound waiting on the future and
not record the watch execution. Recording the watch execution results in
error but is inconsequential to this test.

Fixes #66392

[elasticmachine]

Pinging @elastic/es-data-management (Team:Data Management)

[martijnvg]

I'm not against unmuting this test, but I do wonder whether we can replace it with a unit test instead.

This test tests a particular error that existed before. I wonder whether we can make TransportExecuteWatchAction
unit testable, so that we can test the scenario that the ExecuteWatchQueedStatsTests is testing. Maybe we can raise the visibility of executeWatch() to package protected and create a test instance of ExecutionService (seems like we do this in ExecutionServiceTests), TriggerService (seems like we create test instances of this class in several places), Clock and ThreadPool (using TestThreadPool). The other dependencies can be ignored.

[jakelandis]

but I do wonder whether we can replace it with a unit test instead.

That would probably be best .... However, I think we should try this fix and if it is still flaky then look to replace the test.

[martijnvg]

Makes sense. LGTM

…

On Tue, Jan 4, 2022 at 5:14 PM Jake Landis ***@***.***> wrote: but I do wonder whether we can replace it with a unit test instead. That would probably be best .... However, I think we should try this fix and if it is still flaky then look to replace the test. — Reply to this email directly, view it on GitHub <#82134 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAENWRMLNR4SSOGBX3Q3OWLUUMMG3ANCNFSM5K6V3BLQ> . You are receiving this because your review was requested.Message ID: ***@***.***>