Reapply "[Response Ops][Task Manager] Setting task status directly to running in mget claim strategy
#192303
Conversation
… `running` in `mget` claim strategy (elastic#191669)" This reverts commit 689f227.
ymao1
changed the title
running in mget claim strategy
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#6893[✅] x-pack/test/task_manager_claimer_mget/config.ts: 200/200 tests passed. |
Flaky Test Runner Stats🟠 Some tests failed. - kibana-flaky-test-suite-runner#6895[❌] x-pack/test/task_manager_claimer_mget/config.ts: 174/200 tests passed. |
|
@elasticmachine merge upstream |
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#6897[✅] x-pack/test/task_manager_claimer_mget/config.ts: 100/100 tests passed. |
ymao1
added
release_note:skip
|
Pinging @elastic/response-ops (Team:ResponseOps) |
| @@ -223,7 +227,9 @@ export class TaskManagerRunner implements TaskRunner { | |||
| * @param id | |||
| */ | |||
| public isSameTask(executionId: string) { | |||
| return executionId.startsWith(this.id); | |||
| const executionIdParts = executionId.split('::'); | |||
| const executionIdCompare = executionIdParts.length > 0 ? executionIdParts[0] : executionId; | |||
There was a problem hiding this comment.
I think String::split() will always returns an array with at least one element, in this case, so this could be re-written as:
const executionIdCompare = executionIdParts[0];
OTOH, you'd have to know that, otherwise you might think this is buggy, and the existing code in the PR will actually work fine, so ... not sure if we should really change it :-)
|
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @ymao1 |
Re-doing this PR: #191669
Reverted because it was causing a flaky test. After a lot of investigation, it looks like the flakiness was caused by interference from long-running tasks scheduled as part of other tests. The task partitions test uses task IDs
1,2and3and the tasks were being short circuited when there were other tasks with UUIDs that started with1,2or3due to the logic in the task runner that tries to prevent duplicate recurring tasks from running. That logic just usedstartsWithto test for duplicates where the identifier is${task.id}::${task.executionUUID}. Updated that logic instead to check for duplicatetask.idinstead of just usingstartsWithin this commit: 1646ae9