Set mget task claim strategy as the default (elastic#197070)

Resolves elastic#194625 In this PR, I'm setting `mget` as the default task claiming strategy along the following changes: - Given we no longer need the 8.16 specific PRs (elastic#196317 and elastic#196757), I've also reverted them. - Given we now use `met` as the default, I've renamed `task_manager_claimer_mget` to `task_manager_claimer_update_by_query` and made tests in that folder test using the `update_by_query` claim strategy. - Stabilize flaky tests caused by mget + polling for tasks more frequently Flaky test runners: - [[59b71bc](elastic@59b71bc)] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7197 - [[aea910e](elastic@aea910e)] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7199 - [[4723ced](elastic@4723ced)] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7206 - [[d28c8c5](elastic@d28c8c5)] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7209 - [[dd7773a](elastic@dd7773a)] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7224 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
pmuellr · Oct 25, 2024 · c31f11e · c31f11e
1 parent 4798c59
commit c31f11e
Show file tree

Hide file tree

Showing 44 changed files with 128 additions and 405 deletions.
diff --git a/.buildkite/ftr_platform_stateful_configs.yml b/.buildkite/ftr_platform_stateful_configs.yml
@@ -342,7 +342,7 @@ enabled:
   - x-pack/test/spaces_api_integration/security_and_spaces/config_trial.ts
   - x-pack/test/spaces_api_integration/security_and_spaces/copy_to_space_config_trial.ts
   - x-pack/test/spaces_api_integration/spaces_only/config.ts
-  - x-pack/test/task_manager_claimer_mget/config.ts
+  - x-pack/test/task_manager_claimer_update_by_query/config.ts
   - x-pack/test/ui_capabilities/security_and_spaces/config.ts
   - x-pack/test/ui_capabilities/spaces_only/config.ts
   - x-pack/test/upgrade_assistant_integration/config.ts

diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -745,7 +745,7 @@ x-pack/plugins/runtime_fields @elastic/kibana-management
 packages/kbn-safer-lodash-set @elastic/kibana-security
 x-pack/test/security_api_integration/plugins/saml_provider @elastic/kibana-security
 x-pack/test/plugin_api_integration/plugins/sample_task_plugin @elastic/response-ops
-x-pack/test/task_manager_claimer_mget/plugins/sample_task_plugin_mget @elastic/response-ops
+x-pack/test/task_manager_claimer_update_by_query/plugins/sample_task_plugin_mget @elastic/response-ops
 test/plugin_functional/plugins/saved_object_export_transforms @elastic/kibana-core
 test/plugin_functional/plugins/saved_object_import_warnings @elastic/kibana-core
 x-pack/test/saved_object_api_integration/common/plugins/saved_object_test_plugin @elastic/kibana-security
@@ -1495,7 +1495,7 @@ x-pack/plugins/cloud_integrations/cloud_full_story/server/config.ts @elastic/kib
 /x-pack/test/alerting_api_integration/observability @elastic/obs-ux-management-team
 /x-pack/test/plugin_api_integration/test_suites/task_manager/ @elastic/response-ops
 /x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/ @elastic/response-ops
-/x-pack/test/task_manager_claimer_mget/ @elastic/response-ops
+/x-pack/test/task_manager_claimer_update_by_query/ @elastic/response-ops
 /docs/user/alerting/ @elastic/response-ops
 /docs/management/connectors/ @elastic/response-ops
 /x-pack/test/cases_api_integration/ @elastic/response-ops

diff --git a/config/serverless.yml b/config/serverless.yml
@@ -207,7 +207,6 @@ uiSettings:
     labs:dashboard:deferBelowFold: false
 
 # Task Manager
-xpack.task_manager.claim_strategy: mget
 xpack.task_manager.allow_reading_invalid_state: false
 xpack.task_manager.request_timeouts.update_by_query: 60000
 xpack.task_manager.metrics_reset_interval: 120000

diff --git a/package.json b/package.json
@@ -763,7 +763,7 @@
     "@kbn/safer-lodash-set": "link:packages/kbn-safer-lodash-set",
     "@kbn/saml-provider-plugin": "link:x-pack/test/security_api_integration/plugins/saml_provider",
     "@kbn/sample-task-plugin": "link:x-pack/test/plugin_api_integration/plugins/sample_task_plugin",
-    "@kbn/sample-task-plugin-mget": "link:x-pack/test/task_manager_claimer_mget/plugins/sample_task_plugin_mget",
+    "@kbn/sample-task-plugin-update-by-query": "link:x-pack/test/task_manager_claimer_update_by_query/plugins/sample_task_plugin_mget",
     "@kbn/saved-object-export-transforms-plugin": "link:test/plugin_functional/plugins/saved_object_export_transforms",
     "@kbn/saved-object-import-warnings-plugin": "link:test/plugin_functional/plugins/saved_object_import_warnings",
     "@kbn/saved-object-test-plugin": "link:x-pack/test/saved_object_api_integration/common/plugins/saved_object_test_plugin",

diff --git a/tsconfig.base.json b/tsconfig.base.json
@@ -1484,8 +1484,8 @@
       "@kbn/saml-provider-plugin/*": ["x-pack/test/security_api_integration/plugins/saml_provider/*"],
       "@kbn/sample-task-plugin": ["x-pack/test/plugin_api_integration/plugins/sample_task_plugin"],
       "@kbn/sample-task-plugin/*": ["x-pack/test/plugin_api_integration/plugins/sample_task_plugin/*"],
-      "@kbn/sample-task-plugin-mget": ["x-pack/test/task_manager_claimer_mget/plugins/sample_task_plugin_mget"],
-      "@kbn/sample-task-plugin-mget/*": ["x-pack/test/task_manager_claimer_mget/plugins/sample_task_plugin_mget/*"],
+      "@kbn/sample-task-plugin-update-by-query": ["x-pack/test/task_manager_claimer_update_by_query/plugins/sample_task_plugin_mget"],
+      "@kbn/sample-task-plugin-update-by-query/*": ["x-pack/test/task_manager_claimer_update_by_query/plugins/sample_task_plugin_mget/*"],
       "@kbn/saved-object-export-transforms-plugin": ["test/plugin_functional/plugins/saved_object_export_transforms"],
       "@kbn/saved-object-export-transforms-plugin/*": ["test/plugin_functional/plugins/saved_object_export_transforms/*"],
       "@kbn/saved-object-import-warnings-plugin": ["test/plugin_functional/plugins/saved_object_import_warnings"],

diff --git a/x-pack/plugins/task_manager/server/config.test.ts b/x-pack/plugins/task_manager/server/config.test.ts
@@ -14,6 +14,7 @@ describe('config validation', () => {
       Object {
         "allow_reading_invalid_state": true,
         "auto_calculate_default_ech_capacity": false,
+        "claim_strategy": "mget",
         "discovery": Object {
           "active_nodes_lookback": "30s",
           "interval": 10000,
@@ -44,7 +45,7 @@ describe('config validation', () => {
             "warn_threshold": 80,
           },
         },
-        "poll_interval": 3000,
+        "poll_interval": 500,
         "request_capacity": 1000,
         "request_timeouts": Object {
           "update_by_query": 30000,
@@ -66,7 +67,7 @@ describe('config validation', () => {
     expect(() => {
       configSchema.validate(config);
     }).toThrowErrorMatchingInlineSnapshot(
-      `"The specified monitored_stats_required_freshness (100) is invalid, as it is below the poll_interval (3000)"`
+      `"The specified monitored_stats_required_freshness (100) is invalid, as it is below the poll_interval (500)"`
     );
   });
 
@@ -76,6 +77,7 @@ describe('config validation', () => {
       Object {
         "allow_reading_invalid_state": true,
         "auto_calculate_default_ech_capacity": false,
+        "claim_strategy": "mget",
         "discovery": Object {
           "active_nodes_lookback": "30s",
           "interval": 10000,
@@ -106,7 +108,7 @@ describe('config validation', () => {
             "warn_threshold": 80,
           },
         },
-        "poll_interval": 3000,
+        "poll_interval": 500,
         "request_capacity": 1000,
         "request_timeouts": Object {
           "update_by_query": 30000,
@@ -136,6 +138,7 @@ describe('config validation', () => {
       Object {
         "allow_reading_invalid_state": true,
         "auto_calculate_default_ech_capacity": false,
+        "claim_strategy": "mget",
         "discovery": Object {
           "active_nodes_lookback": "30s",
           "interval": 10000,
@@ -171,7 +174,7 @@ describe('config validation', () => {
             "warn_threshold": 80,
           },
         },
-        "poll_interval": 3000,
+        "poll_interval": 500,
         "request_capacity": 1000,
         "request_timeouts": Object {
           "update_by_query": 30000,

diff --git a/x-pack/plugins/task_manager/server/config.ts b/x-pack/plugins/task_manager/server/config.ts
@@ -202,7 +202,7 @@ export const configSchema = schema.object(
       max: 100,
       min: 1,
     }),
-    claim_strategy: schema.maybe(schema.string()),
+    claim_strategy: schema.string({ defaultValue: CLAIM_STRATEGY_MGET }),
     request_timeouts: requestTimeoutsConfig,
     auto_calculate_default_ech_capacity: schema.boolean({ defaultValue: false }),
   },

diff --git a/...k/plugins/task_manager/server/integration_tests/task_manager_switch_task_claimers.test.ts b/...k/plugins/task_manager/server/integration_tests/task_manager_switch_task_claimers.test.ts
@@ -52,13 +52,13 @@ describe('switch task claiming strategies', () => {
     jest.clearAllMocks();
   });
 
-  it('should switch from default to mget and still claim tasks', async () => {
+  it('should switch from default to update_by_query and still claim tasks', async () => {
     const setupResultDefault = await setupTestServers();
     const esServer = setupResultDefault.esServer;
     let kibanaServer = setupResultDefault.kibanaServer;
     let taskClaimingOpts: TaskClaimingOpts = TaskClaimingMock.mock.calls[0][0];
 
-    expect(taskClaimingOpts.strategy).toBe('update_by_query');
+    expect(taskClaimingOpts.strategy).toBe('mget');
 
     mockTaskTypeRunFn.mockImplementation(() => {
       return { state: {} };
@@ -90,17 +90,17 @@ describe('switch task claiming strategies', () => {
       await kibanaServer.stop();
     }
 
-    const setupResultMget = await setupKibanaServer({
+    const setupResultUbq = await setupKibanaServer({
       xpack: {
         task_manager: {
-          claim_strategy: 'mget',
+          claim_strategy: 'update_by_query',
         },
       },
     });
-    kibanaServer = setupResultMget.kibanaServer;
+    kibanaServer = setupResultUbq.kibanaServer;
 
     taskClaimingOpts = TaskClaimingMock.mock.calls[1][0];
-    expect(taskClaimingOpts.strategy).toBe('mget');
+    expect(taskClaimingOpts.strategy).toBe('update_by_query');
 
     // inject a task to run and ensure it is claimed and run
     const id2 = uuidV4();
@@ -132,19 +132,19 @@ describe('switch task claiming strategies', () => {
     }
   });
 
-  it('should switch from mget to default and still claim tasks', async () => {
-    const setupResultMget = await setupTestServers({
+  it('should switch from update_by_query to default and still claim tasks', async () => {
+    const setupResultUbq = await setupTestServers({
       xpack: {
         task_manager: {
-          claim_strategy: 'mget',
+          claim_strategy: 'update_by_query',
         },
       },
     });
-    const esServer = setupResultMget.esServer;
-    let kibanaServer = setupResultMget.kibanaServer;
+    const esServer = setupResultUbq.esServer;
+    let kibanaServer = setupResultUbq.kibanaServer;
     let taskClaimingOpts: TaskClaimingOpts = TaskClaimingMock.mock.calls[0][0];
 
-    expect(taskClaimingOpts.strategy).toBe('mget');
+    expect(taskClaimingOpts.strategy).toBe('update_by_query');
 
     mockTaskTypeRunFn.mockImplementation(() => {
       return { state: {} };
@@ -180,7 +180,7 @@ describe('switch task claiming strategies', () => {
     kibanaServer = setupResultDefault.kibanaServer;
 
     taskClaimingOpts = TaskClaimingMock.mock.calls[1][0];
-    expect(taskClaimingOpts.strategy).toBe('update_by_query');
+    expect(taskClaimingOpts.strategy).toBe('mget');
 
     // inject a task to run and ensure it is claimed and run
     const id2 = uuidV4();
@@ -212,13 +212,13 @@ describe('switch task claiming strategies', () => {
     }
   });
 
-  it('should switch from default to mget and claim tasks that were running during shutdown', async () => {
+  it('should switch from default to update_by_query and claim tasks that were running during shutdown', async () => {
     const setupResultDefault = await setupTestServers();
     const esServer = setupResultDefault.esServer;
     let kibanaServer = setupResultDefault.kibanaServer;
     let taskClaimingOpts: TaskClaimingOpts = TaskClaimingMock.mock.calls[0][0];
 
-    expect(taskClaimingOpts.strategy).toBe('update_by_query');
+    expect(taskClaimingOpts.strategy).toBe('mget');
 
     mockTaskTypeRunFn.mockImplementation(async () => {
       await new Promise((resolve) => setTimeout(resolve, 2000));
@@ -252,17 +252,17 @@ describe('switch task claiming strategies', () => {
       await kibanaServer.stop();
     }
 
-    const setupResultMget = await setupKibanaServer({
+    const setupResultUbq = await setupKibanaServer({
       xpack: {
         task_manager: {
-          claim_strategy: 'mget',
+          claim_strategy: 'update_by_query',
         },
       },
     });
-    kibanaServer = setupResultMget.kibanaServer;
+    kibanaServer = setupResultUbq.kibanaServer;
 
     taskClaimingOpts = TaskClaimingMock.mock.calls[1][0];
-    expect(taskClaimingOpts.strategy).toBe('mget');
+    expect(taskClaimingOpts.strategy).toBe('update_by_query');
 
     // task doc should still exist and be running
     const task = await kibanaServer.coreStart.elasticsearch.client.asInternalUser.get<{
@@ -290,19 +290,19 @@ describe('switch task claiming strategies', () => {
     }
   });
 
-  it('should switch from mget to default and claim tasks that were running during shutdown', async () => {
-    const setupResultMget = await setupTestServers({
+  it('should switch from update_by_query to default and claim tasks that were running during shutdown', async () => {
+    const setupResultUbq = await setupTestServers({
       xpack: {
         task_manager: {
-          claim_strategy: 'mget',
+          claim_strategy: 'update_by_query',
         },
       },
     });
-    const esServer = setupResultMget.esServer;
-    let kibanaServer = setupResultMget.kibanaServer;
+    const esServer = setupResultUbq.esServer;
+    let kibanaServer = setupResultUbq.kibanaServer;
     let taskClaimingOpts: TaskClaimingOpts = TaskClaimingMock.mock.calls[0][0];
 
-    expect(taskClaimingOpts.strategy).toBe('mget');
+    expect(taskClaimingOpts.strategy).toBe('update_by_query');
 
     mockTaskTypeRunFn.mockImplementation(async () => {
       await new Promise((resolve) => setTimeout(resolve, 2000));
@@ -340,7 +340,7 @@ describe('switch task claiming strategies', () => {
     kibanaServer = setupResultDefault.kibanaServer;
 
     taskClaimingOpts = TaskClaimingMock.mock.calls[1][0];
-    expect(taskClaimingOpts.strategy).toBe('update_by_query');
+    expect(taskClaimingOpts.strategy).toBe('mget');
 
     // task doc should still exist and be running
     const task = await kibanaServer.coreStart.elasticsearch.client.asInternalUser.get<{

diff --git a/x-pack/plugins/task_manager/server/integration_tests/task_state_validation.test.ts b/x-pack/plugins/task_manager/server/integration_tests/task_state_validation.test.ts
@@ -305,7 +305,7 @@ describe('task state validation', () => {
 
     it('should fail the task run when setting allow_reading_invalid_state:false and reading an invalid state', async () => {
       const logSpy = jest.spyOn(pollingLifecycleOpts.logger, 'warn');
-      const updateSpy = jest.spyOn(pollingLifecycleOpts.taskStore, 'bulkUpdate');
+      const updateSpy = jest.spyOn(pollingLifecycleOpts.taskStore, 'bulkPartialUpdate');
 
       const id = uuidV4();
       await injectTask(kibanaServer.coreStart.elasticsearch.client.asInternalUser, {
@@ -332,8 +332,7 @@ describe('task state validation', () => {
         const found = calls.map((arr) => arr[0]).find((message) => message.match(expected) != null);
         expect(found).toMatch(expected);
         expect(updateSpy).toHaveBeenCalledWith(
-          expect.arrayContaining([expect.objectContaining({ id, taskType: 'fooType' })]),
-          { validate: false }
+          expect.arrayContaining([expect.objectContaining({ id })])
         );
       });
     });