refactor: introducing tasks for map operations#3167
Conversation
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
|
Motivation or any doc to understand this change? |
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Signed-off-by: Vigith Maurice <vigith@gmail.com>
Signed-off-by: Yashash <yashashhl25@gmail.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3167 +/- ##
==========================================
+ Coverage 80.08% 80.27% +0.18%
==========================================
Files 297 297
Lines 67717 67602 -115
==========================================
+ Hits 54232 54265 +33
+ Misses 12935 12789 -146
+ Partials 550 548 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Updated the description. |
…tch and stream implementations to prevent insertion after error broadcasting Signed-off-by: Vaibhav Tiwari <vaibhav.tiwari33@gmail.com>
Signed-off-by: Vigith Maurice <vigith@gmail.com>
| while let Some(resp) = resp_stream.next().await { | ||
| match resp { | ||
| Ok(resp) => { | ||
| if let Some(map::TransmissionStatus { eot: true }) = resp.status { |
There was a problem hiding this comment.
nit: After we encounter an error, the remaining Ok responses are processed here. Then in process_response we try to acquire lock to the map only to find out that the map is empty. Should we drain the stream in Err(e) => ... branch?
There was a problem hiding this comment.
I don't think there will be a case of Ok response followed by an Err response.
There was a problem hiding this comment.
It is possible in the current implementation on the server side (Rust SDK at least). The flow responsible for shutting down the server on error is independent of the flow processing incoming requests.
There was a problem hiding this comment.
server logs showing continued sends on stream after panic happens:
�[2m2026-02-04T02:43:17.658992Z�[0m �[31mERROR�[0m �[2mnumaflow::map�[0m�[2m:�[0m Failed to run map function: JoinError::Panic(Id(102), "BypassCat panicked!", ...)
�[2m2026-02-04T02:43:17.658966Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- sent processed response for msg id: 1770172997641487083-4
�[2m2026-02-04T02:43:17.658998Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- Received gRPC request: Ok(..)
�[2m2026-02-04T02:43:17.659008Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- sending panic information to error channel
�[2m2026-02-04T02:43:17.659068Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- Received gRPC request: Ok(..)
�[2m2026-02-04T02:43:17.659075Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- sent processed response for msg id: 1770172997641502453-4
�[2m2026-02-04T02:43:17.659063Z�[0m �[31mERROR�[0m �[2mnumaflow::map�[0m�[2m:�[0m Shutting down gRPC channel: GrpcStatus(Status { code: Internal, ...})
�[2m2026-02-04T02:43:17.659098Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- sent processed response for msg id: 1770172997641513273-4
�[2m2026-02-04T02:43:17.659108Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- stream response tx closed
�[2m2026-02-04T02:43:17.659063Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- sent processed response for msg id: 1770172997641501343-4
�[2m2026-02-04T02:43:17.659085Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- Received gRPC request: Ok(..)
�[2m2026-02-04T02:43:17.659076Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- sent processed response for msg id: 1770172997641503573-4
�[2m2026-02-04T02:43:17.659195Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m Cancellation token is cancelled, shutting down
�[2m2026-02-04T02:43:17.659197Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- sent processed response for msg id: 1770172997641515613-4
�[2m2026-02-04T02:43:17.659201Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- stream response tx closed
�[2m2026-02-04T02:43:17.659201Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- sent processed response for msg id: 1770172997641516783-4
�[2m2026-02-04T02:43:17.659216Z�[0m �[32m INFO�[0m �[2mnumaflow::map�[0m�[2m:�[0m debug -- sent processed response for msg id: 1770172997641517913-4
There was a problem hiding this comment.
Do you see that on the client side where we get an Ok response followed by an Err response?
There was a problem hiding this comment.
I can add another log for checking this out.
Refactors map operation handling by introducing a task-based architecture:
process_concurrent_messages()andprocess_batch_messages()methods, reducing duplication between unary and stream handling.