Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add functionality for user to register close callback #795

Merged
merged 9 commits into from
Oct 28, 2021

Conversation

pentschev
Copy link
Member

@pentschev pentschev commented Oct 18, 2021

This is a similar feature to tornado.BaseIOStream.set_close_callback and allows the application to immediately find out when the remote endpoint has closed or failed. This can be useful in places such as Distributed, where we want to still be able to read messages that are already in-transit, but the endpoint isn't anymore valid for sending messages.

When a user callback is registered, the Endpoint error callback or its
finalizer will run the callback to inform the user's application that
the Endpoint is terminating and will not accept sending new messages,
although receiving may still be possible as UCP may still have some
incoming messages in transit.
@pentschev pentschev marked this pull request as ready for review October 27, 2021 14:29
@pentschev pentschev requested a review from a team as a code owner October 27, 2021 14:29
Since ep.close() calls worker.progress() it can't be called within the
listener callback, therefore we have to progress outside until the
callback is called. We must check the processes' exitcodes to be `0`, if
they timeout the exitcode is `None`, which can't be checked correctly
just with `not process.exitcode`.
Copy link
Member

@madsbk madsbk left a comment

Choose a reason for hiding this comment

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

Nice feature @pentschev, I only have some minor suggestions

ucp/_libs/ucx_endpoint.pyx Outdated Show resolved Hide resolved
ucp/_libs/ucx_endpoint.pyx Outdated Show resolved Hide resolved
ucp/core.py Show resolved Hide resolved
tests/test_endpoint.py Outdated Show resolved Hide resolved
@pentschev
Copy link
Member Author

Thanks @madsbk for the review, I think I addressed everything. Could you take another look when you have the chance?

Copy link
Member

@madsbk madsbk left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @pentschev!

@pentschev
Copy link
Member Author

Thanks @madsbk !

@pentschev pentschev merged commit addee5e into rapidsai:branch-0.23 Oct 28, 2021
@pentschev pentschev deleted the ep-close-user-callback branch October 28, 2021 13:58
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.

2 participants