Skip to content

Conversation

@jayshrivastava
Copy link
Collaborator

@jayshrivastava jayshrivastava commented Sep 25, 2025

This new type of streams allows you to "chain" streams together. This will be used to "chain" FlightData
containing metrics to the stream corresponding to the last executed partition in a task.

Streams already offer a chain() API but it lacks a feature we need - a callback to decide whether
or not to collect and append metrics.

When a stream is complete, as a part of the proposed metrics protocol, the ArrowFlightEndpoint needs to check if
this is the last stream/partition. This synchronization point between N streams for N partitions will
have to be managed in some shared state between the stream and the ArrowFlightEndpoint. In this callback, we
can check if this is the last partition. If so, we can collect metrics and send them in the trailing stream.
For more details, see the draft implementation here

One alternative option considered was to have the callback generate FlightData which can be sent on
the stream. Ultimately, it seemed cleaner to just return another stream.

Informs: #123

@jayshrivastava jayshrivastava force-pushed the js/metrics-emitting branch 2 times, most recently from 3fca767 to 3832545 Compare September 25, 2025 18:59
This new type of streams allows you to "chain" streams together. This will be used to "chain" FlightData
containing metrics to the stream corresponding to the last executed partition in a task.

Streams already offer a `chain()` API but it lacks a feature we need - a callback to decide whether
or not to collect and append metrics.

When a stream is complete, as a part of the proposed metrics protocol, the ArrowFlightEndpoint needs to check if
this is the last stream/partition. This synchronization point between N streams for N partitions will
have to be managed in some shared state between the stream and the ArrowFlightEndpoint. In this callback, we
can check if this is the last partition. If so, we can collect metrics and send them in the trailing stream.
For more details, see the draft implementation [here](https://github.com/datafusion-contrib/datafusion-distributed/pull/139/files#diff-fa3e517ceea7f93b2d50873bcdf7f48f6110a5cf8b25a4a8df338f7d71dc6fdb)

One alternative option considered was to have the callback generate `FlightData` which can be sent on
the stream. Ultimately, it seemed cleaner to just return another stream.

Informs: #123
@jayshrivastava jayshrivastava marked this pull request as ready for review September 25, 2025 19:01
Copy link
Collaborator

@gabotechs gabotechs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 nice building block!

@jayshrivastava jayshrivastava merged commit 2df3467 into main Sep 26, 2025
4 checks passed
@jayshrivastava jayshrivastava deleted the js/metrics-emitting branch September 26, 2025 13:50
@jayshrivastava
Copy link
Collaborator Author

TYFR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants