kita-renji commented on code in PR #51498:
URL: https://github.com/apache/arrow/pull/51498#discussion_r4118482751
##########
cpp/src/arrow/util/async_generator.h:
##########
@@ -1338,8 +1363,11 @@ class MergedGenerator {
// Now we have given up the lock and we can take all the actions we
decided we
// need to take.
+ if (signaled_error) {
+ RunErrorSignaledHookForTesting();
+ }
if (should_mark_final_error) {
- state->MarkFinalError(maybe_next->status(), std::move(sink));
+ state->DeliverFinalError(maybe_next->status(), std::move(sink));
Review Comment:
Thanks for the careful review and the repro steps.
Fixed in 9a48233e6:
- `SetFinalErrorUnlocked` now attaches the waiting caller's error callback
to `all_finished` inside the same locked section that sets `broken`, on both
the inner and outer error paths. Any pull that observes `broken` therefore
registers its `IterationEnd` continuation after the error callback.
`DeliverFinalError` is gone. `all_finished` can't be finished at that point,
since the failed request is still counted in `outstanding_requests`, so the
callback never runs under the lock. `operator()` already attaches its terminal
`Then()` to `all_finished` while holding the same mutex.
- Added `InnerErrorToWaiterNotOvertakenByLaterPull` and
`OuterErrorToWaiterNotOvertakenByLaterPull`, following your steps: one caller
waiting, a second pull from the hook, then fail the pending future and check
the completion order. Both fail on the previous commit with `{"terminal",
"error"}` and pass now.
- On the non-blocking point: the hook is now a private static member, and
the `MergedGeneratorErrorHookTest` fixture is a friend, the same pattern as
`key_hash_internal.h` and `chunker_internal.h`. It's no longer part of the
public API.
All 133 tests in `arrow-async-utility-test` pass locally, and the
`MergedGenerator` tests passed 200/200 with `--gtest_repeat`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
