[8.x] Fix bug in calculating when ad-hoc tasks are out of attempts (#…

…192907) (#192918)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Fix bug in calculating when ad-hoc tasks are out of attempts
(#192907)](#192907)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Mike
Côté","email":"mikecote@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-09-13T18:40:29Z","message":"Fix
bug in calculating when ad-hoc tasks are out of attempts (#192907)\n\nIn
this PR, I'm fixing a bug where ad-hoc tasks would have one
fewer\r\nattempts to retry in failure scenarios when using
mget.\r\n\r\n## To verify\r\n\r\n1. Apply the following diff to your
code\r\n```\r\ndiff --git
a/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts
b/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts\r\nindex
0275b2bdc2f..d481c3820a1 100644\r\n---
a/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts\r\n+++
b/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts\r\n@@
-77,6 +77,10 @@ export function getConnectorType():
ServerLogConnectorType {\r\n async function executor(\r\n execOptions:
ServerLogConnectorTypeExecutorOptions\r\n ):
Promise<ConnectorTypeExecutorResult<void>> {\r\n+\r\n+ console.log('***
Server log execution');\r\n+ throw new Error('Fail');\r\n+\r\n const {
actionId, params, logger } = execOptions;\r\n\r\n const sanitizedMessage
= withoutControlCharacters(params.message);\r\ndiff --git
a/x-pack/plugins/task_manager/server/config.ts
b/x-pack/plugins/task_manager/server/config.ts\r\nindex
db07494ef4f..07e277f8d16 100644\r\n---
a/x-pack/plugins/task_manager/server/config.ts\r\n+++
b/x-pack/plugins/task_manager/server/config.ts\r\n@@ -202,7 +202,7 @@
export const configSchema = schema.object(\r\n max: 100,\r\n min: 1,\r\n
}),\r\n- claim_strategy: schema.string({ defaultValue:
CLAIM_STRATEGY_UPDATE_BY_QUERY }),\r\n+ claim_strategy: schema.string({
defaultValue: CLAIM_STRATEGY_MGET }),\r\n request_timeouts:
requestTimeoutsConfig,\r\n },\r\n {\r\ndiff --git
a/x-pack/plugins/task_manager/server/lib/get_retry_at.ts
b/x-pack/plugins/task_manager/server/lib/get_retry_at.ts\r\nindex
278ba18642d..c8fb911d500 100644\r\n---
a/x-pack/plugins/task_manager/server/lib/get_retry_at.ts\r\n+++
b/x-pack/plugins/task_manager/server/lib/get_retry_at.ts\r\n@@ -54,6
+54,7 @@ export function getRetryDate({\r\n }\r\n\r\n export function
calculateDelayBasedOnAttempts(attempts: number) {\r\n+ return 10 *
1000;\r\n // Return 30s for the first retry attempt\r\n if (attempts ===
1) {\r\n return 30 * 1000;\r\n```\r\n2. Create an always firing rule
that runs every hour, triggering a\r\nserver log on check
intervals\r\n3. Let the rule run and observe the server log action
running and\r\nfailing three times (as compared to
two)","sha":"36eedc121bff8d83fc6a4590f468397f56d0bd14","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Feature:Task
Manager","Team:ResponseOps","v9.0.0","backport:prev-minor","v8.16.0"],"title":"Fix
bug in calculating when ad-hoc tasks are out of
attempts","number":192907,"url":"https://github.com/elastic/kibana/pull/192907","mergeCommit":{"message":"Fix
bug in calculating when ad-hoc tasks are out of attempts (#192907)\n\nIn
this PR, I'm fixing a bug where ad-hoc tasks would have one
fewer\r\nattempts to retry in failure scenarios when using
mget.\r\n\r\n## To verify\r\n\r\n1. Apply the following diff to your
code\r\n```\r\ndiff --git
a/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts
b/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts\r\nindex
0275b2bdc2f..d481c3820a1 100644\r\n---
a/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts\r\n+++
b/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts\r\n@@
-77,6 +77,10 @@ export function getConnectorType():
ServerLogConnectorType {\r\n async function executor(\r\n execOptions:
ServerLogConnectorTypeExecutorOptions\r\n ):
Promise<ConnectorTypeExecutorResult<void>> {\r\n+\r\n+ console.log('***
Server log execution');\r\n+ throw new Error('Fail');\r\n+\r\n const {
actionId, params, logger } = execOptions;\r\n\r\n const sanitizedMessage
= withoutControlCharacters(params.message);\r\ndiff --git
a/x-pack/plugins/task_manager/server/config.ts
b/x-pack/plugins/task_manager/server/config.ts\r\nindex
db07494ef4f..07e277f8d16 100644\r\n---
a/x-pack/plugins/task_manager/server/config.ts\r\n+++
b/x-pack/plugins/task_manager/server/config.ts\r\n@@ -202,7 +202,7 @@
export const configSchema = schema.object(\r\n max: 100,\r\n min: 1,\r\n
}),\r\n- claim_strategy: schema.string({ defaultValue:
CLAIM_STRATEGY_UPDATE_BY_QUERY }),\r\n+ claim_strategy: schema.string({
defaultValue: CLAIM_STRATEGY_MGET }),\r\n request_timeouts:
requestTimeoutsConfig,\r\n },\r\n {\r\ndiff --git
a/x-pack/plugins/task_manager/server/lib/get_retry_at.ts
b/x-pack/plugins/task_manager/server/lib/get_retry_at.ts\r\nindex
278ba18642d..c8fb911d500 100644\r\n---
a/x-pack/plugins/task_manager/server/lib/get_retry_at.ts\r\n+++
b/x-pack/plugins/task_manager/server/lib/get_retry_at.ts\r\n@@ -54,6
+54,7 @@ export function getRetryDate({\r\n }\r\n\r\n export function
calculateDelayBasedOnAttempts(attempts: number) {\r\n+ return 10 *
1000;\r\n // Return 30s for the first retry attempt\r\n if (attempts ===
1) {\r\n return 30 * 1000;\r\n```\r\n2. Create an always firing rule
that runs every hour, triggering a\r\nserver log on check
intervals\r\n3. Let the rule run and observe the server log action
running and\r\nfailing three times (as compared to
two)","sha":"36eedc121bff8d83fc6a4590f468397f56d0bd14"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192907","number":192907,"mergeCommit":{"message":"Fix
bug in calculating when ad-hoc tasks are out of attempts (#192907)\n\nIn
this PR, I'm fixing a bug where ad-hoc tasks would have one
fewer\r\nattempts to retry in failure scenarios when using
mget.\r\n\r\n## To verify\r\n\r\n1. Apply the following diff to your
code\r\n```\r\ndiff --git
a/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts
b/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts\r\nindex
0275b2bdc2f..d481c3820a1 100644\r\n---
a/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts\r\n+++
b/x-pack/plugins/stack_connectors/server/connector_types/server_log/index.ts\r\n@@
-77,6 +77,10 @@ export function getConnectorType():
ServerLogConnectorType {\r\n async function executor(\r\n execOptions:
ServerLogConnectorTypeExecutorOptions\r\n ):
Promise<ConnectorTypeExecutorResult<void>> {\r\n+\r\n+ console.log('***
Server log execution');\r\n+ throw new Error('Fail');\r\n+\r\n const {
actionId, params, logger } = execOptions;\r\n\r\n const sanitizedMessage
= withoutControlCharacters(params.message);\r\ndiff --git
a/x-pack/plugins/task_manager/server/config.ts
b/x-pack/plugins/task_manager/server/config.ts\r\nindex
db07494ef4f..07e277f8d16 100644\r\n---
a/x-pack/plugins/task_manager/server/config.ts\r\n+++
b/x-pack/plugins/task_manager/server/config.ts\r\n@@ -202,7 +202,7 @@
export const configSchema = schema.object(\r\n max: 100,\r\n min: 1,\r\n
}),\r\n- claim_strategy: schema.string({ defaultValue:
CLAIM_STRATEGY_UPDATE_BY_QUERY }),\r\n+ claim_strategy: schema.string({
defaultValue: CLAIM_STRATEGY_MGET }),\r\n request_timeouts:
requestTimeoutsConfig,\r\n },\r\n {\r\ndiff --git
a/x-pack/plugins/task_manager/server/lib/get_retry_at.ts
b/x-pack/plugins/task_manager/server/lib/get_retry_at.ts\r\nindex
278ba18642d..c8fb911d500 100644\r\n---
a/x-pack/plugins/task_manager/server/lib/get_retry_at.ts\r\n+++
b/x-pack/plugins/task_manager/server/lib/get_retry_at.ts\r\n@@ -54,6
+54,7 @@ export function getRetryDate({\r\n }\r\n\r\n export function
calculateDelayBasedOnAttempts(attempts: number) {\r\n+ return 10 *
1000;\r\n // Return 30s for the first retry attempt\r\n if (attempts ===
1) {\r\n return 30 * 1000;\r\n```\r\n2. Create an always firing rule
that runs every hour, triggering a\r\nserver log on check
intervals\r\n3. Let the rule run and observe the server log action
running and\r\nfailing three times (as compared to
two)","sha":"36eedc121bff8d83fc6a4590f468397f56d0bd14"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

x-pack/plugins/task_manager/server/task_running/task_runner.test.ts

@@ -2304,6 +2304,32 @@ describe('TaskManagerRunner', () => {
 
       expect(runner.isAdHocTaskAndOutOfAttempts).toEqual(true);
     });
+
+    it(`should return true if attempts = max attempts and in claiming status`, async () => {
+      const { runner } = await pendingStageSetup({
+        instance: {
+          id: 'foo',
+          taskType: 'testbar',
+          attempts: 5,
+          status: TaskStatus.Claiming,
+        },
+      });
+
+      expect(runner.isAdHocTaskAndOutOfAttempts).toEqual(true);
+    });
+
+    it(`should return false if attempts = max attempts and in running status`, async () => {
+      const { runner } = await pendingStageSetup({
+        instance: {
+          id: 'foo',
+          taskType: 'testbar',
+          attempts: 5,
+          status: TaskStatus.Running,
+        },
+      });
+
+      expect(runner.isAdHocTaskAndOutOfAttempts).toEqual(false);
+    });
   });
 
   describe('removeTask()', () => {

x-pack/plugins/task_manager/server/task_running/task_runner.ts

@@ -294,6 +294,10 @@ export class TaskManagerRunner implements TaskRunner {
    * running a task, the task should be deleted instead of ran.
    */
   public get isAdHocTaskAndOutOfAttempts() {
+    if (this.instance.task.status === 'running') {
+      // This function gets called with tasks marked as running when using MGET, so attempts is already incremented
+      return !this.instance.task.schedule && this.instance.task.attempts > this.getMaxAttempts();
+    }
     return !this.instance.task.schedule && this.instance.task.attempts >= this.getMaxAttempts();
   }