Watcher: Mark watcher as started only after loading watches #30403

spinscale · 2018-05-04T22:44:18Z

Starting watcher should wait for the watches to be started before
marking the status as started.

Also, reloading watcher could set the execution service to paused. This could
lead to watches not being executed, when run in tests. This fix does not
change the paused flag in the execution service, just clears out the
current queue and executions.

Some tests were affected by this because after putting a watch, the
execution service was still marked as paused, so that the manual
execution with the ScheduleEngineTriggerMock did not work.

Closes #30381

Starting watcher should wait for the watches to be started before marking the status as started. Also, reloading watcher could set the execution service to paused. This could lead to watches not being executed, when run in tests. This fix does not change the paused flag in the execution service, just clears out the current queue and executions. Some tests were affected by this because after putting a watch, the execution service was still marked as paused, so that the manual execution with the ScheduleEngineTriggerMock did not work. Closes elastic#30381

elasticmachine · 2018-05-04T22:44:19Z

Pinging @elastic/es-core-infra

…ion-service-state

bleskes

LGTM. Thanks for explaining. I left some nits and we discussed some possible clarifications in the code.

bleskes · 2018-05-07T07:35:22Z

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherService.java

+                if (reloadInner(state, "starting", true)) {
+                    callback.run();
+                }
+            },
            e -> logger.error("error starting watcher", e)));


do we want to do something with is error? (not related to this change)

bleskes · 2018-05-07T08:14:35Z

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/WatcherService.java

+     * @param state    the current cluster state
+     * @param callback the callback to be triggered, when watches where loaded successfully
+     */
+    public void start(ClusterState state, Runnable callback) {


can we call the callback onStarted ? it is not called on error

bleskes · 2018-05-07T08:20:23Z

...plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/execution/ExecutionService.java

@@ -121,11 +121,19 @@ public void unPause() {
    }

    /**
-     * Pause the execution of the watcher executor
+     * Pause the execution of the watcher executor, and empty the state


Can you unpack the java docs to explain what this does? this seems too terse. For example it doesn't mention reload?

…ion-service-state

Starting watcher should wait for the watcher to be started before marking the status as started, which is now done via a callback. Also, reloading watcher could set the execution service to paused. This could lead to watches not being executed, when run in tests. This fix does not change the paused flag in the execution service, just clears out the current queue and executions. Closes #30381

…r-you * origin/master: Update forcemerge.asciidoc (elastic#30113) Added zentity to the list of API extension plugins (elastic#29143) Fix the search request default operation behavior doc (elastic#29302) (elastic#29405) Watcher: Mark watcher as started only after loading watches (elastic#30403) Pass the task to broadcast actions (elastic#29672) Disable REST default settings testing until elastic#29229 is back-ported Correct wording in log message (elastic#30336) Do not fail snapshot when deleting a missing snapshotted file (elastic#30332) AwaitsFix testCreateShrinkIndexToN DOCS: Correct mapping tags in put-template api DOCS: Fix broken link in the put index template api Add put index template api to high level rest client (elastic#30400) Relax testAckedIndexing to allow document updating [Docs] Add snippets for POS stop tags default value Move respect accept header on no handler to 6.3.1

* origin/master: (39 commits) Docs: fix changelog merge Fix line length violation in cache tests Add stricter geohash parsing (elastic#30376) Add failing test for core cache deadlock [DOCS] convert forcemerge snippet Update forcemerge.asciidoc (elastic#30113) Added zentity to the list of API extension plugins (elastic#29143) Fix the search request default operation behavior doc (elastic#29302) (elastic#29405) Watcher: Mark watcher as started only after loading watches (elastic#30403) Pass the task to broadcast actions (elastic#29672) Disable REST default settings testing until elastic#29229 is back-ported Correct wording in log message (elastic#30336) Do not fail snapshot when deleting a missing snapshotted file (elastic#30332) AwaitsFix testCreateShrinkIndexToN DOCS: Correct mapping tags in put-template api DOCS: Fix broken link in the put index template api Add put index template api to high level rest client (elastic#30400) Relax testAckedIndexing to allow document updating [Docs] Add snippets for POS stop tags default value Move respect accept header on no handler to 6.3.1 ...

* elastic-master: Watcher: Mark watcher as started only after loading watches (#30403) Pass the task to broadcast actions (#29672) Disable REST default settings testing until #29229 is back-ported Correct wording in log message (#30336) Do not fail snapshot when deleting a missing snapshotted file (#30332) AwaitsFix testCreateShrinkIndexToN DOCS: Correct mapping tags in put-template api DOCS: Fix broken link in the put index template api Add put index template api to high level rest client (#30400) Relax testAckedIndexing to allow document updating [Docs] Add snippets for POS stop tags default value Move respect accept header on no handler to 6.3.1 Respect accept header on no handler (#30383) [Test] Add analysis-nori plugin to the vagrant tests [Docs] Fix bad link [Docs] Fix end of section in the korean plugin docs Expose the Lucene Korean analyzer module in a plugin (#30397) Docs: remove transport_client from CCS role example (#30263) [Rollup] Validate timezone in range queries (#30338) Use readFully() to read bytes from CipherInputStream (#28515) Fix docs Recently merged #29229 had a doc bug that broke the doc build. This commit fixes. Test: remove cluster permission from CCS user (#30262) Add Get Settings API support to java high-level rest client (#29229) Watcher: Remove unneeded index deletion in tests

Starting watcher should wait for the watcher to be started before marking the status as started, which is now done via a callback. Also, reloading watcher could set the execution service to paused. This could lead to watches not being executed, when run in tests. This fix does not change the paused flag in the execution service, just clears out the current queue and executions. Closes #30381

* 6.x: Stop forking javac (#30462) Fix tribe tests Docs: Use task_id in examples of tasks (#30436) Security: Rename IndexLifecycleManager to SecurityIndexManager (#30442) Packaging: Set elasticsearch user to have non-existent homedir (#29007) [Docs] Fix typo in cardinality-aggregation.asciidoc (#30434) Avoid NPE in `more_like_this` when field has zero tokens (#30365) Build: Switch to building javadoc with html5 (#30440) Add a quick tour of the project to CONTRIBUTING (#30187) Add stricter geohash parsing (#30376) Reindex: Use request flavored methods (#30317) Silence SplitIndexIT.testSplitIndexPrimaryTerm test failure. (#30432) Auto-expand replicas when adding or removing nodes (#30423) Silence IndexUpgradeIT test failures. (#30430) Fix line length violation in cache tests Add failing test for core cache deadlock [DOCS] convert forcemerge snippet Update forcemerge.asciidoc (#30113) Added zentity to the list of API extension plugins (#29143) Fix the search request default operation behavior doc (#29302) (#29405) Watcher: Mark watcher as started only after loading watches (#30403) Correct wording in log message (#30336) Do not fail snapshot when deleting a missing snapshotted file (#30332) AwaitsFix testCreateShrinkIndexToN DOCS: Correct mapping tags in put-template api DOCS: Fix broken link in the put index template api Add put index template api to high level rest client (#30400) [Docs] Add snippets for POS stop tags default value Remove entry inadvertently picked into changelog Move respect accept header on no handler to 6.3.1 Respect accept header on no handler (#30383) [Test] Add analysis-nori plugin to the vagrant tests [Rollup] Validate timezone in range queries (#30338) [Docs] Fix bad link [Docs] Fix end of section in the korean plugin docs add the Korean nori plugin to the change logs Expose the Lucene Korean analyzer module in a plugin (#30397) Docs: remove transport_client from CCS role example (#30263) Test: remove cluster permission from CCS user (#30262) Watcher: Remove unneeded index deletion in tests fix docs branch version fix lucene snapshot version Upgrade to 7.4.0-snapshot-1ed95c097b (#30357) [ML][TEST] Clean up jobs in ModelPlotIT Watcher: Ensure trigger service pauses execution (#30363) [DOCS] Fixes ordering of changelog sections [DOCS] Commented out empty sections in the changelog to fix the doc build. (#30372) Make RepositoriesMetaData contents unmodifiable (#30361) Change signature of Get Repositories Response (#30333) 6.x Backport: Terms query validate bug (#30319) InternalEngineTests.testConcurrentOutOfOrderDocsOnReplica should use two documents (#30121) Security: reduce garbage during index resolution (#30180) Test: use trial license in qa tests with security [ML] Add integration test for model plots (#30359) SQL: Fix bug caused by empty composites (#30343) [ML] Account for gaps in data counts after job is reopened (#30294) [ML] Refactor DataStreamDiagnostics to use array (#30129) Make licensing FIPS-140 compliant (#30251) Do not load global state when deleting a snapshot (#29278) Don't load global state when only restoring indices (#29239) Tests: Use different watch ids per test in smoke test (#30331) Watcher: Make start/stop cycle more predictable and synchronous (#30118) [Docs] Add term query with normalizer example Adds Eclipse config for xpack licence headers (#30299) Fix message content in users tool (#30293) [DOCS] Removed X-Pack breaking changes page [DOCS] Added security breaking change [DOCS] Fixes link to TLS LDAP info [DOCS] Merges X-Pack release notes into changelog (#30350) [DOCS] Fixes broken links to bootstrap user (#30349) [Docs] Remove errant changelog line Fix NPE when CumulativeSum agg encounters null/empty bucket (#29641) [DOCS] Reorganizes authentication details in Stack Overview (#30280) Tests: Simplify VersionUtils released version splitting (#30322) Fix merging logic of Suggester Options (#29514) ReplicationTracker.markAllocationIdAsInSync may hang if allocation is cancelled (#30316) [DOCS] Adds LDAP realm configuration details (#30214) [DOCS] Adds native realm configuration details (#30215) Disable SSL on testing old BWC nodes (#30337) [DOCS] Enables edit links for X-Pack pages Cancelling a peer recovery on the source can leak a primary permit (#30318) SQL: Reduce number of ranges generated for comparisons (#30267) [DOCS] Adds links to changelog sections Convert server javadoc to html5 (#30279) REST Client: Add Request object flavored methods (#29623) Create default ES_TMPDIR on Windows (#30325) [Docs] Clarify `fuzzy_like_this` redirect (#30183) Fix docs of the `_ignored` meta field. Add a new `_ignored` meta field. (#29658) Move repository-azure fixture test to QA project (#30253)

spinscale added >test Issues or PRs that are addressing/adding tests review v7.0.0 :Data Management/Watcher v6.4.0 labels May 4, 2018

spinscale added 2 commits May 7, 2018 08:42

Merge branch 'master' into 1805-fix-test-failures-due-to-wrong-execut…

5754636

…ion-service-state

add starting state when starting up to share this info in watcher status

1035fb2

bleskes approved these changes May 7, 2018

View reviewed changes

spinscale added 2 commits May 7, 2018 10:49

Merge branch 'master' into 1805-fix-test-failures-due-to-wrong-execut…

28e5ca8

…ion-service-state

review comments

211bec7

spinscale merged commit 71ebed6 into elastic:master May 7, 2018

spinscale mentioned this pull request May 11, 2018

[CI] watcher_state: expected [started] but was [starting] #30298

Closed

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Watcher: Mark watcher as started only after loading watches #30403

Watcher: Mark watcher as started only after loading watches #30403

spinscale commented May 4, 2018

elasticmachine commented May 4, 2018

bleskes left a comment

bleskes May 7, 2018

bleskes May 7, 2018

bleskes May 7, 2018

Watcher: Mark watcher as started only after loading watches #30403

Watcher: Mark watcher as started only after loading watches #30403

Conversation

spinscale commented May 4, 2018

elasticmachine commented May 4, 2018

bleskes left a comment

Choose a reason for hiding this comment

bleskes May 7, 2018

Choose a reason for hiding this comment

bleskes May 7, 2018

Choose a reason for hiding this comment

bleskes May 7, 2018

Choose a reason for hiding this comment