Handle stopped task queue in table task cleanup tests#17720
Handle stopped task queue in table task cleanup tests#17720xiangfu0 wants to merge 1 commit intoapache:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates PinotTableRestletResourceTest to reduce flakiness in testTableTasksCleanupWithActiveTasks by making minion-task state checks and deletion cleanup more race-tolerant when tables are deleted with ignoreActiveTasks=true.
Changes:
- Added helpers to wait for a task to reach a state or be missing, and to force-delete tasks with retries.
- Updated the active-task cleanup test to stop the minion task queue before waiting/deleting the task.
| sendPutRequest(DEFAULT_INSTANCE.getControllerRequestURLBuilder() | ||
| .forStopMinionTaskQueue(MinionConstants.SegmentGenerationAndPushTask.TASK_TYPE)); | ||
| waitForTaskStateOrTaskMissing(taskName, TaskState.STOPPED); | ||
| deleteMinionTaskWithRetry(taskName); |
There was a problem hiding this comment.
The test stops the minion task queue but never resumes it. This can leak shared state into subsequent tests in this class/suite (note the earlier cleanup test explicitly resumes the queue “to avoid affecting other tests”). Please ensure the queue is resumed (ideally in a finally block or in @AfterMethod cleanup) even if assertions fail.
| private static boolean isTaskStateNotFound(IOException e) { | ||
| String message = e.getMessage(); | ||
| return message != null && (message.contains("status code: 404") || message.contains("Not Found") | ||
| || message.contains("does not exist")); | ||
| } |
There was a problem hiding this comment.
isTaskStateNotFound detects 404s by substring-matching IOException.getMessage(), which is brittle and may misclassify errors if message formats change. Since the request helpers wrap HttpErrorStatusException as the IOException cause, prefer checking e.getCause() for HttpErrorStatusException and using its status code (or a dedicated helper that returns status codes) to reliably detect 404s.
❌ 5 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
afba597 to
2aca9c1
Compare
Summary
Validation
Not run (not requested); this is a test-only stability change for flaky test coverage.