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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17720 +/- ##
=========================================
Coverage 63.21% 63.21%
- Complexity 1501 1502 +1
=========================================
Files 3179 3181 +2
Lines 190717 190816 +99
Branches 29154 29164 +10
=========================================
+ Hits 120566 120631 +65
- Misses 60779 60807 +28
- Partials 9372 9378 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
xiangfu0
force-pushed
the
codex/fix-flaky-table-task-cleanup
branch
from
afba597 to
2aca9c1
Compare
xiangfu0
changed the title
xiangfu0
force-pushed
the
codex/fix-flaky-table-task-cleanup
branch
from
6a1f4e1 to
ffaaf9a
Compare
Summary
@BeforeMethodto resumeSegmentGenerationAndPushTaskbefore scheduling any test task.getOrScheduleTask(...)that:finallywhere it is explicitly stopped/deleted.Validation
Not run (not requested); this is a test-only stability fix targeting flaky test setup in
PinotTableRestletResourceTest.