[core][taskEvents out of GCS][3/n] Export task events from aggregator agent to dashboard head - #65028
[core][taskEvents out of GCS][3/n] Export task events from aggregator agent to dashboard head#65028karticam wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the GCS-based task events publisher with a new HTTP-based publisher (AsyncDashboardHeadPublisherClient) that sends events directly to the dashboard head. It also introduces a new TaskEventsHead module to receive and temporarily buffer these events in memory. Feedback on the changes suggests setting a maximum length on the in-memory event buffer (collections.deque) in TaskEventsHead to prevent potential out-of-memory (OOM) issues from unbounded growth.
| # TODO(Task 3): replace this unbounded in-memory buffer with real storage + | ||
| # GC/eviction. It exists only so the ingestion endpoint has somewhere to put | ||
| # events for now; nothing reads it yet (State-API serving is a later task). | ||
| self._events = collections.deque() |
There was a problem hiding this comment.
The in-memory buffer self._events is currently unbounded. In a long-running cluster with high task throughput, this will lead to unbounded memory growth and eventually cause the dashboard head process to be OOM-killed.
Even though this is a temporary placeholder before real storage is implemented, it is highly recommended to set a reasonable maxlen on the deque (e.g., 100000) as a defensive safeguard to prevent memory exhaustion.
| self._events = collections.deque() | |
| self._events = collections.deque(maxlen=100000) |
- New TaskEventsHead SubprocessModule exposing POST /api/task_events - Deserialization isolated in _deserialize_request (wire format TBD) - In-memory buffer stub; storage/GC is a later task Signed-off-by: Kartica Modi <karticamodi@gmail.com>
Signed-off-by: Kartica Modi <karticamodi@gmail.com>
Signed-off-by: Kartica Modi <karticamodi@gmail.com>
Signed-off-by: Kartica Modi <karticamodi@gmail.com>
Signed-off-by: Kartica Modi <karticamodi@gmail.com>
Signed-off-by: Kartica Modi <karticamodi@gmail.com>
a98d6cc to
f632c5e
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit f632c5e. Configure here.
| return dashboard_optional_utils.rest_response( | ||
| status_code=dashboard_utils.HTTPStatusCode.INTERNAL_ERROR, | ||
| message=f"Failed to deserialize task events request: {e}", | ||
| ) |
There was a problem hiding this comment.
Wrong status for bad payload
Low Severity
Deserialization failures for /api/task_events return HTTPStatusCode.INTERNAL_ERROR (500). A malformed client body is a client error, so this misclassifies bad input as a server failure, unlike nearby handlers such as EventHead.report_events which treat parse failures as bad requests.
Reviewed by Cursor Bugbot for commit f632c5e. Configure here.


Part of the effort to move task-event observability data out of GCS and onto the
dashboard head. This PR adds support for moving task events from aggregator agent to dashboard head. Changes are as follows:
task_events_headthat hosts a POST/api/task_eventsendpoint to receive task events from the aggregator agent. For now it just buffers received events in memory. More stuff to be done in upcoming PRs.AsyncDashboardHeadPublisherClientand a corresponding flag toPUBLISH_EVENTS_TO_DASHBOARD_HEAD.AsyncGCSTaskEventsPublisherClientis still kept since some tests depend on it. This will be removed later after the entire migration completes.AsyncGCSTaskEventsPublisherClientin that it has the same event selection to export and has the same proto building, but sends the request over HTTP POST instead of a gRPC call.TaskEventsMetadataBuffer.get()is destructive and a shared buffer would split the metadata across publishers.gcs_client, then makes the POST request. If the address isn't registered yet the publish is marked unsuccessful and the publisher retries on its next cycle; once resolved, the endpoint is cached.