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 about BeforeRecv #28

Closed
HypenZou opened this issue Dec 28, 2021 · 9 comments
Closed

question about BeforeRecv #28

HypenZou opened this issue Dec 28, 2021 · 9 comments

Comments

@HypenZou
Copy link

HypenZou commented Dec 28, 2021

个人对BeforeRecv这个接口有点疑问, 这个接口在一次recv过程中会被调用两次, 类似:

for {
    BeforeRecv()
    conn.Read(buf)
}

会循环两次, 第二次先调用BeforeRecv, 然后读套接字, 这时才被阻塞。个人感觉这是个bug?

@lesismal
Copy link
Owner

lesismal commented Dec 28, 2021

没太懂你意思,是指 for 循环中每次 read 之前都去调用 BeforeRecv 吗?如果是这个意思,那这个不是bug,对 conn 的处理通常包括两个地方:
一是建立连接时:可以通过HandleConnected处理新连接,这里通常用于设置一些conn基础参数,比如tcp read/write buffer size,这个设置后就不需要再更新设置了
二是对于 keepalive 的需求、需要经常更新超时时间,BeforeRecv主要是用于处理这个需求,心跳超时比较好的实现是每个连接每次读取一个完整协议包之前设置超时时间,也就是这里的 for 循环内的逻辑。其他一些框架也有使用单独的goroutine定时遍历所有连接、根据最后一次读取时间判断是否超时,这样会多消耗一个协程,并且如果在线量很大遍历成本有点高,而且对于每个连接都是不精确的

@lesismal
Copy link
Owner

这里再多说一下 keepalive:
4层tcp也有keepalive的选项可以设置,但是4层的keepalive无法满足7层的需要,比如僵尸连接连到服务器上,tcp keepalive一直是健康的、4层协议本身一直保持连接不断开,但是这个僵尸连接什么都不做、空耗服务器资源。
所以7层通常仍需要自己进行心跳的逻辑、从而及时踢掉僵尸连接。

@HypenZou
Copy link
Author

HypenZou commented Dec 28, 2021

谢谢回复.

  1. 我的意思是在这个地方:

    arpc/client.go

    Line 723 in c3f8d6d

    msg, err = c.Handler.Recv(c)

    for循环里调用Recv(), 每个协议包读完之后,会重新进入循环, 再次调用Recv()里的beforeRecv, 之后才会被阻塞, 这样beforeRecv貌似会被调用两次?
  2. 我也不太理解为什么keepAlive要在BeforeRecv里做而不在handler里做😂, 我感觉常见的都是每个连接维护一个timer, 在handler里更新每个连接的idletime;用AfterFunc做个回调检验是否到期,到期了关闭,没到期继续调用AfterFunc 。这样也不需要用单独goroutine定时遍历啊🧐

@lesismal
Copy link
Owner

lesismal commented Dec 28, 2021

for循环里调用Recv(), 每个协议包读完之后,会重新进入循环, 再次调用Recv()里的beforeRecv, 之后才会被阻塞, 这样beforeRecv貌似会被调用两次?
不是两次,而是 recv 多少次,就调用多少次。就像 BeforeRecv 的名字一样,就是要在 conn Recv 前调用。但并不是强制用户这样用,所以 beforeRecv 默认是放空的参数

我也不太理解为什么keepAlive要在BeforeRecv里做而不在handler里做😂, 我感觉常见的都是每个连接维护一个timer, 在handler里更新每个连接的idletime;
如果你的handler是指读到一个完整包后的处理函数/handler,那是不可以的,因为如果僵尸连接,根本没机会读到一个完整协议包

用AfterFunc做个回调检验是否到期,到期了关闭,没到期继续调用AfterFunc 。这样也不需要用单独goroutine定时遍历啊🧐
标准库 TCPConn 的 SetDeadline 好像是最终到这里:
https://github.com/golang/go/blob/b357b05b70d2b8c4988ac2a27f2af176e7a09e1b/src/internal/poll/fd_poll_runtime.go#L160
https://github.com/golang/go/blob/b357b05b70d2b8c4988ac2a27f2af176e7a09e1b/src/runtime/netpoll.go#L253

runtime 里的也是timer,但是估计只是超时后原来 Recv 阻塞的协程里直接失败、这是runtime调度就可控的,不需要额外创建一个协程来执行 Close 之类的,而 time.AfterFunc 在超时后的回调是异步、需要创建一个协程,这个成本是不划算的:
https://github.com/golang/go/blob/master/src/time/sleep.go#L167

而且退一步讲,既然 TCPConn 已经提供了 SetDeadline 相关,go 这种一个连接一个协程的同步代码的便利性,也不需要time.AfterFunc那么麻烦

@HypenZou
Copy link
Author

package main

import (
	"log"
	"net"
	"time"

	"github.com/lesismal/arpc"
)

func main() {
	svr := arpc.NewServer()
	svr.Handler.BeforeRecv(func(c net.Conn) error {
		log.Printf("BeforeRecv 调用")
		return nil
	})
	svr.Handler.Handle("/echo/sync", func(ctx *arpc.Context) {
		str := ""
		ctx.Bind(&str)
		ctx.Write(str)
	})
	go func() {
		time.Sleep(time.Second)
		client, err := arpc.NewClient(func() (net.Conn, error) {
			return net.DialTimeout("tcp", "localhost:8888", time.Second*3)
		})
		req := "hello"
		rsp := ""
		err = client.Call("/echo/sync", &req, &rsp, time.Second*500)
		if err != nil {
			log.Printf("Call /echo/sync failed: %v", err)
		} else {
			log.Printf("Call /echo/sync Response: \"%v\"", rsp)
		}
	}()
	svr.Run("localhost:8888")
}
/*
2021/12/29 00:49:55.560 [INF] [ARPC SVR] Running On: "127.0.0.1:8888"
2021/12/29 00:49:56.560 [INF] [ARPC CLI]        127.0.0.1:8888  Connected
2021/12/29 00:49:56.560 [INF] [ARPC SVR]        127.0.0.1:34924 Connected
2021/12/29 00:49:56 BeforeRecv 调用
2021/12/29 00:49:56 BeforeRecv 调用
2021/12/29 00:49:56 Call /echo/sync Response: "hello"
*/

比如上面的代码, 我觉得在recv结束之后, BeforeRecv 被再次调用了吧.

arpc/client.go

Line 722 in c3f8d6d

for c.running {

当这个client.Conn不可读的时候, 这个循环会先调用BeforeRecv, 再被阻塞

@lesismal
Copy link
Owner

比如上面的代码, 我觉得在recv结束之后, BeforeRecv 被再次调用了吧.

这有什么问题吗?每次读取完整协议包之前都应该按照当前时间更新deadline,如果只设置一次,比如:

setdeadline(15s)
for {
    read
}

这样15s后、不管有没有数据过来,读都会失败。

@landbed
Copy link

landbed commented Dec 29, 2021

逻辑上没问题,demo是日志打印顺序的问题,并不是执行顺序的问题,因为拿到Resp并打印比内部for循环跑的晚。

@HypenZou
Copy link
Author

HypenZou commented Jan 1, 2022

比如上面的代码, 我觉得在recv结束之后, BeforeRecv 被再次调用了吧.

这有什么问题吗?每次读取完整协议包之前都应该按照当前时间更新deadline,如果只设置一次,比如:

setdeadline(15s)
for {
    read
}

这样15s后、不管有没有数据过来,读都会失败。

哦哦, 是我想错了, 打扰了
新年快乐!

@HypenZou HypenZou closed this as completed Jan 1, 2022
@lesismal
Copy link
Owner

lesismal commented Jan 1, 2022

比如上面的代码, 我觉得在recv结束之后, BeforeRecv 被再次调用了吧.

这有什么问题吗?每次读取完整协议包之前都应该按照当前时间更新deadline,如果只设置一次,比如:

setdeadline(15s)
for {
    read
}

这样15s后、不管有没有数据过来,读都会失败。

哦哦, 是我想错了, 打扰了 新年快乐!

新年快乐!:smile:

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

No branches or pull requests

3 participants