MLH-1239 Tags propagate isolate #5379
Conversation
sriram-atlan
requested review from
aarshi0301,
hr2904,
muniatl,
nikhilbonte21 and
suraj5077
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } else { | ||
| LOG.info("TasksFetcher: No task to queue"); | ||
| if (submittedCount > 0) { | ||
| LOG.info("TasksFetcher: Submitted {} tasks to the queue", submittedCount); |
There was a problem hiding this comment.
Bug: Task Submission and Latch Decrement Mismatch
The CountDownLatch is initialized with the total task count, but submitAll skips null tasks or exits early without decrementing it. This prevents the latch from reaching zero, causing waitForTasksToComplete to hang. Also, the new task submission retry loop lacks a maximum retry limit, risking an infinite loop if memory stays high.
| Response.ResponseBuilder builder; | ||
|
|
||
| // Return 503 if processing tasks or high memory | ||
| if (isProcessingTasks || memoryUsage > 0.75) { |
There was a problem hiding this comment.
Bug: Inconsistent Memory Thresholds Across Components
The new /admin/ready endpoint's memory threshold for readiness is hardcoded to 0.75. This creates an inconsistency with the configurable AtlasConfiguration.TASK_MEMORY_THRESHOLD_PERCENT used by other components like TaskQueueWatcher, potentially causing different behavior between the readiness check and task processing.
| submitAll(tasks, latch); | ||
| LOG.info("Submitted {} tasks to the queue", tasks.size()); | ||
| waitForTasksToComplete(latch); | ||
| isProcessingTasks.set(true); |
There was a problem hiding this comment.
This logic just responsible for submittiing tasks to the queue.
IMO it should never tell whether isProcessingTasks, best please for this flag would be the TaskExecutor.run() method.
Also, generally we do check RequestContext.get().getCurrentTask(); to conclude whether any task is in progress or not, may be same can be used instead the flag but this way also fine
Change description
Type of change
Related issues
https://atlanhq.atlassian.net/browse/MLH-1239
Helm Config Changes for Running Tests (Staging PR)
Does this PR require Helm config changes for testing?
enpla9up36. (You can proceed with the PR.) ✅Checklists
Development
Security
Code review