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

It is recommended that the GRPC server add a timeout limit. #3189

Open
bitker opened this issue Dec 5, 2023 · 3 comments
Open

It is recommended that the GRPC server add a timeout limit. #3189

bitker opened this issue Dec 5, 2023 · 3 comments
Labels
feature help wanted planned This issue/proposal is planned into our next steps.

Comments

@bitker
Copy link

bitker commented Dec 5, 2023

建议GRPC服务端加入超时限制。
防止某些情况下导致服务端执行耗时阻塞任务时,客户端长时间链接导致资源浪费。
某些场景:如服务端在执行CURL任务时可能会有很久的延时(当然是开发者没有设置超时限制)。这样会导致很多客户端一直等待。
个人经历:某次开发时使用了gcahe中的GetOrSetFuncLock这个函数,由于此函数中又执行了CURL请求,最后导致了大批量并发链接等待。当时我个人原因没有注意到此函数会阻塞等待,导致了客户端很多链接在等待处理(经查发现等待了半小时没有返回)。后来切换gozero后没有出现长时等待。经复盘发现gozero的服务端是有超时设置
可能很多人会说客户端链接时可以设置ctx的超时控制。我也知道这个,但是如果有个别开发者粗心没有设置此超时的话将会导致大量链接等待了。
gozero的服务端超时控制是在中间件中将ctx上下重新设置超时控制。以下是部分gozero的超时控制流程。
希望哪位大佬 能改个加入gf的grpc就完美了

type (
	// MethodTimeoutConf defines specified timeout for gRPC method.
	MethodTimeoutConf struct {
		FullMethod string
		Timeout    time.Duration
	}

	methodTimeouts map[string]time.Duration
)

// UnaryTimeoutInterceptor returns a func that sets timeout to incoming unary requests.
func UnaryTimeoutInterceptor(timeout time.Duration,
	methodTimeouts ...MethodTimeoutConf) grpc.UnaryServerInterceptor {
	timeouts := buildMethodTimeouts(methodTimeouts)
	return func(ctx context.Context, req any, info *grpc.UnaryServerInfo,
		handler grpc.UnaryHandler) (any, error) {
		t := getTimeoutByUnaryServerInfo(info.FullMethod, timeouts, timeout)
		ctx, cancel := context.WithTimeout(ctx, t)
		defer cancel()

		var resp any
		var err error
		var lock sync.Mutex
		done := make(chan struct{})
		// create channel with buffer size 1 to avoid goroutine leak
		panicChan := make(chan any, 1)
		go func() {
			defer func() {
				if p := recover(); p != nil {
					// attach call stack to avoid missing in different goroutine
					panicChan <- fmt.Sprintf("%+v\n\n%s", p, strings.TrimSpace(string(debug.Stack())))
				}
			}()

			lock.Lock()
			defer lock.Unlock()
			resp, err = handler(ctx, req)
			close(done)
		}()

		select {
		case p := <-panicChan:
			panic(p)
		case <-done:
			lock.Lock()
			defer lock.Unlock()
			return resp, err
		case <-ctx.Done():
			err := ctx.Err()
			if errors.Is(err, context.Canceled) {
				err = status.Error(codes.Canceled, err.Error())
			} else if errors.Is(err, context.DeadlineExceeded) {
				err = status.Error(codes.DeadlineExceeded, err.Error())
			}
			return nil, err
		}
	}
}

func buildMethodTimeouts(timeouts []MethodTimeoutConf) methodTimeouts {
	mt := make(methodTimeouts, len(timeouts))
	for _, st := range timeouts {
		if st.FullMethod != "" {
			mt[st.FullMethod] = st.Timeout
		}
	}

	return mt
}

func getTimeoutByUnaryServerInfo(method string, timeouts methodTimeouts,
	defaultTimeout time.Duration) time.Duration {
	if v, ok := timeouts[method]; ok {
		return v
	}

	return defaultTimeout
}
@Issues-translate-bot Issues-translate-bot changed the title 建议GRPC服务端加入超时限制。 It is recommended that the GRPC server add a timeout limit. Dec 5, 2023
@gqcn gqcn added feature help wanted planned This issue/proposal is planned into our next steps. labels Oct 7, 2024
Copy link

github-actions bot commented Oct 7, 2024

Hello @bitker. We like your proposal/feedback and would appreciate a contribution via a Pull Request by you or another community member. We thank you in advance for your contribution and are looking forward to reviewing it!
你好 @bitker。我们喜欢您的提案/反馈,并希望您或其他社区成员通过拉取请求做出贡献。我们提前感谢您的贡献,并期待对其进行审查。

@gqcn
Copy link
Member

gqcn commented Oct 7, 2024

@bitker
不好意思,我来晚了,由于邮件丢失我错过了一些issue的消息提示。
这是个非常好的建议!可以以中间件的形式扩展这块能力,其实开发这个超时中间件逻辑也比较简单。grpc通过拦截器的机制有着很好的扩展性,同时goframe框架的gclient也实现了拦截器/中间件的机制,所以框架更优先建设的事扩展性,这些实现可以靠社区来一起建设。如果要实现的话建议grpchttp两者都实现,欢迎大家一起来建设吧。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


@bitker
Sorry, I'm late. I missed some issue messages due to lost emails.
This is very good advice! This capability can be extended in the form of middleware. In fact, it is relatively simple to develop this timeout middleware logic. grpc has good scalability through the interceptor mechanism. At the same time, gclient of the goframe framework also implements the interceptor/middleware mechanism, so the framework gives priority to scalability. These implementations can rely on the community Come build together. If you want to implement it, it is recommended to implement both grpc and http. Everyone is welcome to build it together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature help wanted planned This issue/proposal is planned into our next steps.
Projects
None yet
Development

No branches or pull requests

3 participants