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

new --exit-on-disconnect flag to start lbrynet, will exit process when connection drops #3646

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

eukreign
Copy link
Member

@eukreign eukreign commented Aug 8, 2022

No description provided.

@eukreign eukreign added area: network type: new feature New functionality that does not exist yet labels Aug 8, 2022
@eukreign eukreign changed the title --exit-on-disconnect flag to start lbrynet new --exit-on-disconnect flag to start lbrynet, will exit process when connection drops Aug 8, 2022
@moodyjon
Copy link
Contributor

Can you please rebase or use some other technique to simplify the change? Things like 54da1e3 and 4f9c98e have already been submitted.

@eukreign eukreign force-pushed the exit_on_disconnect branch from 25bf9bd to 8bd7008 Compare August 30, 2022 02:03
@coveralls
Copy link

Coverage Status

Coverage remained the same at 57.78% when pulling 8bd7008 on exit_on_disconnect into a334a93 on master.

Copy link
Contributor

@moodyjon moodyjon left a comment

Choose a reason for hiding this comment

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

General comment about sleep_delay and _urgent_need_reconnect:

    async def network_loop(self):
        sleep_delay = 30
        while self.running:
            await asyncio.wait(
                [asyncio.sleep(30), self._urgent_need_reconnect.wait()], return_when=asyncio.FIRST_COMPLETED   #### It seems like this should be sleep(sleep_delay), or min(sleep_delay, X) not a fixed 30s.
            )
            if self._urgent_need_reconnect.is_set():
                sleep_delay = 30
            self._urgent_need_reconnect.clear()   #### Should this be moved after "if not client:" so that the urgent reconnect will still be needed? When cleared here, the urgency goes away until another RPC is tried and finds the connection is not available.
            if not self.is_connected:
                client = await self.connect_to_fastest()
                if not client:
                    log.warning("failed to connect to any spv servers, retrying later")
                    sleep_delay *= 2
                    sleep_delay = min(sleep_delay, 300)
                    continue
                log.debug("get spv server features %s:%i", *client.server)

Comment on lines +409 to +410
log.error("exiting on server disconnect")
sys.exit(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

You should consider adding SystemExit exception handling here:

except (GracefulExit, KeyboardInterrupt, asyncio.CancelledError):

I guess the risk is that it would hang while shutting down components, but calling daemon.stop() is done in other scenarios like SIGINT, SIGTERM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: network type: new feature New functionality that does not exist yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants