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

Refactor of main to allow graceful shutdown #158

Merged
merged 6 commits into from
Sep 19, 2024

Conversation

r4f4ss
Copy link

@r4f4ss r4f4ss commented Sep 17, 2024

solves #156

@GrapeBaBa
Copy link
Member

Since we forked geth, is there any existing code to reuse or reference.

@@ -45,6 +47,14 @@ type Config struct {
Networks []string
}

type Client struct {
DiscV5API *discover.DiscV5API

Choose a reason for hiding this comment

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

DiscV5API is not needed to close. In fact, all network use the same UDPv5, every network will stop the discv5 when call the Stop method

Copy link
Member

Choose a reason for hiding this comment

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

Since DiscV5 not managed by sub networks, I would like to suggest we don't close discV5 in sub networks close function and close by the outside management object.

Copy link
Author

Choose a reason for hiding this comment

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

I tried to move the closure of DiskV5 from PortalProtocol, where it was originally, to main, but it broke a test and may potentially break other parts of the code that calls PortalProtocol.Close().

DiscV5 close uses a sync.Once struct and can safely be closed many times:

func (t *UDPv5) Close() {

@r4f4ss
Copy link
Author

r4f4ss commented Sep 18, 2024

Since we forked geth, is there any existing code to reuse or reference.

The reference comes from

func StartNode(ctx *cli.Context, stack *node.Node, isConsole bool) {
that geth main function calls to create a new node. Inside this function (StartNode) a goroutine watches a channel for signals of interruption and executes the shutdown function (
shutdown()
).

I studied this file to understand how to handle a syscall.SIGINT or a syscall.SIGTERM signals, but I believe it is not possible to reuse this code to deal with Shisui since geth implements an anonymous goroutine.

Other aspect is the behavior/functionality of CTRL-C command: in geth to force quit is necessary to press 10 times CTRL-C, or panic behavior:

for i := 10; i > 0; i-- {
        <-sigc
        if i > 1 {
          log.Warn("Already shutting down, interrupt more to panic.", "times", i-1)
        }
      }

while in Shisui only a second press of CTRL-c quits.

Please let me know if there is a better approach I should implement.

@fearlessfe fearlessfe merged commit 4a93362 into optimism-java:portal Sep 19, 2024
1 check passed
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.

3 participants