Fetch active task payloads from memory#15377
Fetch active task payloads from memory#15377abhishekagarwal87 merged 3 commits intoapache:masterfrom
Conversation
AmatyaAvadhanula
changed the title
| } | ||
|
|
||
| @Nullable | ||
| Task getActiveTask(String taskId) |
There was a problem hiding this comment.
Instead of Nullable, this can return an Optional.
| { | ||
| return storage.getTask(taskid); | ||
| // Try to fetch active task from memory | ||
| final Task activeTask = taskLockbox.getActiveTask(taskid); |
There was a problem hiding this comment.
I thought this should always be called under the giant lock no ?
There was a problem hiding this comment.
Thanks, I've ensured that the call happens within a lock in the new TaskQueue method
kfaraz
left a comment
kfaraz
left a comment
There was a problem hiding this comment.
This would be nice to have but I am not sure if it is okay to keep the payload of all active tasks in memory. This could significantly increase the memory usage of the Overlord e.g. in case of many concurrent streaming tasks.
|
@kfaraz thank you for your feedback. Instead of maintaining a separate map which could increase the memory usage of the Overlord, this patch now utilizes the map of active tasks that already exists in the TaskQueue |
| return stats; | ||
| } | ||
|
|
||
| public Optional<Task> getTask(String id) |
There was a problem hiding this comment.
I think it would be better for this method to return only active tasks, i.e. it shouldn't have to query the underlying storage on behalf of the caller. As such, this method should be renamed to getActiveTask
The TaskQueue maintains a map of active task ids to tasks, which can be utilized to get active task payloads, before falling back to the metadata store.
The TaskQueue maintains a map of active task ids to tasks, which can be utilized to get active task payloads, before falling back to the metadata store.
5 participants
The TaskQueue maintains a map of active task ids to tasks, which can be utilized to get active task payloads, before falling back to the metadata store.
This PR has: