Skip to content

Conversation

CuriousGeorgiy
Copy link
Member

@CuriousGeorgiy CuriousGeorgiy commented Aug 15, 2025

This patchset fixes some bugs with the Connector wait* methods.

Closes #121
closes #133

@CuriousGeorgiy CuriousGeorgiy force-pushed the wait-methods-fixes branch 5 times, most recently from 75e5c49 to f18a1ba Compare August 15, 2025 20:18
@CuriousGeorgiy CuriousGeorgiy requested review from drewdzzz and mkostoevr and removed request for drewdzzz August 15, 2025 20:19
@CuriousGeorgiy
Copy link
Member Author

@mkostoevr @drewdzzz

In the context of #121 PTAL at #141. Maybe we should fix #141 rather than fixing #121 if it makes sense to you.

@drewdzzz
Copy link
Collaborator

Regarding #141 - it's good idea to do so. But for now, let's simply fix waitAny to conform expected behavior. Who knows, how long will it take to refactor NetProvider::wait machinery and is it possible at all to overcome epoll internal errors.

@drewdzzz drewdzzz assigned CuriousGeorgiy and unassigned mkostoevr and drewdzzz Aug 25, 2025
Let's pull out the code for checking the readiness of requested and count
responses in `waitAll` and `waitCount` correspondingly. We will further use
these helpers to check the futures before waiting.

Needed for tarantool#133
@CuriousGeorgiy CuriousGeorgiy force-pushed the wait-methods-fixes branch 5 times, most recently from 10ba188 to 762d103 Compare September 9, 2025 17:34
Currently, `waitAll` and `waitCount` unconditionally `wait` instead of
checking response readiness. If there are no more responses, it will cause
them to hang indefinitely. To fix this, let's check the response
readiness first. We should also move the time start to the beginning of the
waiting loop, since the initial response checking overhead should not be
accounted for the waiting time.

Closes tarantool#133
Currently, we do not check the return code of `wait` in `waitAny`. If there
is an error, we must log it and return a `nullopt`.

Closes tarantool#121
Copy link
Collaborator

@drewdzzz drewdzzz left a comment

Choose a reason for hiding this comment

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

Thank you for the patch! Glad to see the connector is thriving!
LGTM.

@drewdzzz drewdzzz merged commit 8e009e8 into tarantool:master Sep 16, 2025
142 of 145 checks passed
@CuriousGeorgiy CuriousGeorgiy changed the title Wait methods fixes Wait methods fixes [part 1] Sep 21, 2025
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.

Methods waitCount and waitAll wait even if all futures are ready WaitAny method doesn't check if wait has failed

3 participants