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

[QUESTION] - How to get Listener from server.Default() so that I could use it in different libraries? #545

Open
sujit-baniya opened this issue Jan 15, 2023 · 14 comments
Assignees
Labels
enhancement New feature or request question Further information is requested

Comments

@sujit-baniya
Copy link

Describe the Question

I'm trying to use hertz framework with https://github.com/pedia/endless to allow zero downtime restart. There's an example to include Listener but I'm not sure how to use it with hertz

Reproducible Code

func serve_http(addr string, ln net.Listener) (*fasthttp.Server, error) {
	server := &fasthttp.Server{Handler: func(ctx *fasthttp.RequestCtx) {
		ctx.WriteString(fmt.Sprintf("%d\n", os.Getpid()))
	}}

	go server.Serve(ln)

	return server, nil
}

func main() {
	addr := ":3030"
	var s *fasthttp.Server

	endless.Start(
		func(p *endless.Parent) error {
			ln, err := net.Listen("tcp", addr)
			if err != nil {
				return err
			}

			p.AddListener(ln, addr)
			s, err = serve_http(addr, ln)
			return err
		}, func(c *endless.Child) error {
			nf, ok := c.NamedFiles[addr]
			fmt.Printf("got %#v\n", nf)
			if !ok {
				return fmt.Errorf("inherit %s not found", addr)
			}

			c.AddListener(nf.LN, addr)
			server, err := serve_http(addr, nf.LN)
			s = server
			return err
		},
		func(ctx context.Context) error {
			return s.Shutdown()
		},
	)
	fmt.Printf("Quit %d\n", os.Getpid())
}

Expected behavior

Able to get listener

Screenshots

If applicable, add screenshots to help explain your question.

Hertz version:

Latest

@li-jin-gou li-jin-gou added question Further information is requested enhancement New feature or request labels Jan 15, 2023
@li-jin-gou li-jin-gou self-assigned this Jan 27, 2023
@li-jin-gou
Copy link
Member

I will research it and welcome it together.💕

@sujit-baniya
Copy link
Author

@li-jin-gou Thanks. Please suggest if you've any idea

@welkeyever
Copy link
Member

Seems that you wanna pass a customized listener to Hertz?
Actually it doesn't support that now, but I will looking into it - there isn't much work to do.

@welkeyever welkeyever self-assigned this Feb 2, 2023
@sujit-baniya
Copy link
Author

sujit-baniya commented Feb 2, 2023

@welkeyever Yes and also, I want to get a listener from Hertz and use it for different purposes. So basically when Hertz is set up for Spin if it could store Listener somewhere and I could access that Listener from RequestContext, then that would also work.

@welkeyever
Copy link
Member

@sujit-baniya It's reasonable to store it in network layer of hertz instance - you may get it through transport-related APIs. But it would be strange that relate listener to RequestContext - RequestContext is main for application-related jobs - you may save hertz instance somewhere, and get it to do whatever you wanna later.

@sujit-baniya
Copy link
Author

@welkeyever Yes getting listener from hertz instance would also work. I was giving RequestContext as an example :)

@sujit-baniya
Copy link
Author

Anything like hertzInstance.GetListener() or hertzInstance.Listener would work.

@welkeyever
Copy link
Member

Okay, let me have a try.
I will update if there is any progress.

@sujit-baniya
Copy link
Author

@welkeyever any thought on this issue?

@welkeyever
Copy link
Member

@welkeyever any thought on this issue?

@sujit-baniya Sorry for the late. I was stumped by other things before. I will continue with this part.

@welkeyever
Copy link
Member

Possible solution goes here: develop...welkeyever:hertz:feat/more_options_for_transporter

Please feel free to add your review comments.

The endless example works, however there is a issue related to a third party lib:
image

And we will finally get rid of this lib after this #541.

@sujit-baniya
Copy link
Author

@welkeyever Please let me know your thought if we use Options.Listener net.Listener for Engine and pass it to transporter.

Something like this:

type Options struct {
    ....
    Listener net.Listener
}

func WithListener(ln net.Listener) config.Option {
	return config.Option{F: func(o *config.Options) {
		o.Listener = ln
	}}
}

And for each transport we assign this listener to transport

// For transporter switch
func NewTransporter(options *config.Options) network.Transporter {
	return &transport{
		readBufferSize:   options.ReadBufferSize,
		network:          options.Network,
		addr:             options.Addr,
		keepAliveTimeout: options.KeepAliveTimeout,
		readTimeout:      options.ReadTimeout,
		tls:              options.TLS,
		listenConfig:     options.ListenConfig,
                ln: options.Listener,
		OnAccept:         options.OnAccept,
		OnConnect:        options.OnConnect,
	}
}

And in transport.serve()

    if t.ln == nil {
    network.UnlinkUdsFile(t.network, t.addr) //nolint:errcheck
	t.lock.Lock()
	if t.listenConfig != nil {
		t.ln, err = t.listenConfig.Listen(context.Background(), t.network, t.addr)
	} else {
		t.ln, err = net.Listen(t.network, t.addr)
	}
	t.lock.Unlock()
	if err != nil {
		return err
	}
    }

@sujit-baniya
Copy link
Author

I did above and got the same error as yours

@welkeyever
Copy link
Member

WithListener

Continuously adding options like WithListener will make it confusing when user adding both WithListenConfig and WithListener - there is a potential effective priority under this scenario, but it cannot be well informed to the user.

So I just provider a config-independent way to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Development

No branches or pull requests

3 participants