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

CloseOnShutdown dont work? #1737

Open
modaniru opened this issue Mar 13, 2024 · 3 comments
Open

CloseOnShutdown dont work? #1737

modaniru opened this issue Mar 13, 2024 · 3 comments
Labels

Comments

@modaniru
Copy link

modaniru commented Mar 13, 2024

CloseOnShutdown doesn't work

I have the following code

package kek

import (
	"fmt"
	"github.com/valyala/fasthttp"
	"net"
	"sync"
	"time"
)

func createServer() *fasthttp.Server {
	handler := func(ctx *fasthttp.RequestCtx) {
		path := string(ctx.Path())
		switch path {
		case "/ping":
			fmt.Println(ctx.ConnID())
			time.Sleep(750 * time.Millisecond)
			ctx.Write([]byte("{\"response\":\"ok\"}"))
		default:
			ctx.SetStatusCode(404)
		}
	}
	return &fasthttp.Server{
		CloseOnShutdown: true,
		ReadTimeout:     5 * time.Second,
		IdleTimeout:     10 * time.Second,
		Handler: handler,
		ConnState: func(conn net.Conn, state fasthttp.ConnState) {

		},
	}
}

func createClient() *fasthttp.Client {
	return &fasthttp.Client{
		//ReadTimeout:  5 * time.Second,
		//WriteTimeout: 5 * time.Second,
		//MaxIdleConnDuration: 5 * time.Second,
	}
}

func doRequests(client *fasthttp.Client, n int, url string, errChan chan error) {
	for i := 0; i < n; i++ {
		time.Sleep(1 * time.Millisecond)
		go func() {
			for {
				req := fasthttp.Request{}
				req.SetRequestURI(url)
				resp := fasthttp.Response{}
				err := client.Do(&req, &resp)
				if err != nil {
					errChan <- err
					return
				}

				if resp.ConnectionClose() || resp.Header.ConnectionClose() {
					fmt.Println("conn close")
				}
				time.Sleep(2000 * time.Millisecond)
			}
		}()
	}
}

func main() {
	errMap := make(map[string]int, 10)

	server := createServer()
	go server.ListenAndServe(":80")
	time.Sleep(2 * time.Second)

	client := createClient()
	errChan := make(chan error)

	count := 300
	wg := sync.WaitGroup{}
	wg.Add(count)

	go doRequests(client, count, "http://localhost/ping", errChan)

	go func() {
		timer := time.NewTimer(8000 * time.Millisecond)
		<-timer.C
		fmt.Println("terminating...")
		server.Shutdown()
	}()

	go func() {
		err, ok := <-errChan
		for ok {
			errMap[err.Error()]++
			wg.Done()
			err, ok = <-errChan
		}
	}()

	wg.Wait()
	close(errChan)

	for k, v := range errMap {
		fmt.Printf("error: %s, count - %d\n", k, v)
	}
}

After the server receives a signal from Shutdown(), some connections are closed and do not notify the fasthttp client about it (Do not receive the 'Connection: close' header). As a result, an error like this occurs on the client: "the server closed connection before returning the first response byte. Make sure the server returns 'Connection: close' response header before closing the connection".
Shouldn't this control the CloseOnShutdown field of fasthttp.Server?

@erikdubbelboer
Copy link
Collaborator

When shutting down idle connections are just closed. This is normal. Since there is no request there is also nothing to respond with Connection: close to.

The error you are getting in the client is because the client has no way of knowing if a connection is closed than trying to send something on it. In theory the fasthttp client should ignore this error when re-using an idle connection. But this hasn't been implemented yet.

@modaniru
Copy link
Author

Why can't the server wait until all connections are closed?
I thought this problem could be solved by implementing a Shutdown method, a piece of code that will wait until all connections are closed. If requests come through this connection (during a shutdown), it will send the “Connection: close” header. Thus: connections that are actively used will be closed by the server itself, and inactive ones will simply be closed by timeout (the server closes in the amount of time that we have allocated for the lifetime of the keep-alive connection)

@erikdubbelboer
Copy link
Collaborator

Having the idle connections be closes right away or with a timeout will just give the same results. Http clients should always be able to handle idle connections being closed by the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants