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

Improvement: rename the variables in TCP reconnect function and some default constants #84

Merged
merged 8 commits into from
Jul 16, 2024
50 changes: 28 additions & 22 deletions transport/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"crypto/x509"
"encoding/pem"
"fmt"
"math"
"net"
"os"
"strings"
Expand All @@ -41,10 +42,11 @@ import (
)

const (
reconnectInterval = 3e8 // 300ms
connectInterval = 5e8 // 500ms
connectTimeout = 3e9
maxTimes = 10
defaultReconnectInterval = 3e8 // 300ms
connectInterval = 5e8 // 500ms
connectTimeout = 3e9
defaultMaxReconnectAttempts = 50
maxBackOffTimes = 10
Comment on lines +45 to +49
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Incomplete Renaming of Constants

The constants have been partially renamed to improve readability. However, occurrences of the old constant name reconnectInterval still exist in the codebase. Please ensure all references to the old constant names are updated to the new names.

  • transport/options.go
  • transport/client.go
  • transport/options_test.go
Analysis chain

Renamed Constants for Clarity.

The constants have been renamed to improve readability. Ensure that all references to these constants in the codebase are updated accordingly.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all references to the renamed constants are updated in the codebase.

# Test: Search for the old constant names. Expect: No occurrences of old names.
rg --type go 'reconnectInterval|maxTimes'

Length of output: 783


Script:

#!/bin/bash
# Description: Verify that the new constant names are used correctly in the codebase.

# Test: Search for the new constant names in the codebase.
rg --type go 'defaultReconnectInterval|maxBackOffTimes'

Length of output: 384

)

var (
Expand Down Expand Up @@ -207,14 +209,18 @@ func (c *client) dialUDP() Session {
}

// check connection alive by write/read action
conn.SetWriteDeadline(time.Now().Add(1e9))
if err := conn.SetWriteDeadline(time.Now().Add(1e9)); err != nil {
log.Warnf("failed to set write deadline: %+v", err)
}
if length, err = conn.Write(connectPingPackage[:]); err != nil {
conn.Close()
log.Warnf("conn.Write(%s) = {length:%d, err:%+v}", string(connectPingPackage), length, perrors.WithStack(err))
<-gxtime.After(connectInterval)
continue
}
conn.SetReadDeadline(time.Now().Add(1e9))
if err := conn.SetReadDeadline(time.Now().Add(1e9)); err != nil {
log.Warnf("failed to set read deadline: %+v", err)
}
length, err = conn.Read(buf)
if netErr, ok := perrors.Cause(err).(net.Error); ok && netErr.Timeout() {
err = nil
Expand Down Expand Up @@ -423,33 +429,33 @@ func (c *client) RunEventLoop(newSession NewSessionCallback) {
// a for-loop connect to make sure the connection pool is valid
func (c *client) reConnect() {
var (
num, max, times, interval int
maxDuration int64
sessionNum, reconnectAttempts int
maxReconnectInterval int64
Comment on lines +432 to +433
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Incomplete Variable Renaming

The variables num, times, and maxDuration have not been fully renamed to sessionNum, reconnectAttempts, and maxReconnectInterval respectively in the codebase. The following occurrences in transport/client.go still need to be updated:

  • num:
    • transport/client.go: if c.number <= 0 || c.addr == "" {
    • transport/client.go: panic(fmt.Sprintf("client type:%s, @connNum:%d, @serverAddr:%s", t, c.number, c.addr))
    • transport/client.go: c.ssMap = make(map[Session]struct{}, c.number)
    • transport/client.go: var num int
    • transport/client.go: num = len(c.ssMap)
    • transport/client.go: return num
    • transport/client.go: connPoolSize := c.number

Please ensure that all references to these variables are updated accordingly to maintain consistency.

Analysis chain

Renamed Variables for Clarity.

The variables num, times, and maxDuration have been renamed to sessionNum, reconnectAttempts, and maxReconnectInterval respectively. Ensure that all references to these variables in the codebase are updated accordingly.


The search results indicate that the old variable names num, times, and maxDuration still exist in the codebase. This suggests that not all references have been updated. To verify this, we need to check the specific lines in transport/client.go and other related files to confirm whether the renaming was applied correctly.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all references to the renamed variables are updated in the codebase.

# Test: Search for the old variable names. Expect: No occurrences of old names.
rg --type go 'num|times|maxDuration'

Length of output: 4177


Script:

#!/bin/bash
# Find the specific lines in transport/client.go where the renaming was done
rg 'sessionNum|reconnectAttempts|maxReconnectInterval' transport/client.go

# Check for the old variable names in the context of transport/client.go to confirm if they were renamed
rg 'num|times|maxDuration' transport/client.go

Length of output: 931

)
max = c.number
interval = c.reconnectInterval
if interval == 0 {
interval = reconnectInterval
reconnectInterval := c.reconnectInterval
if reconnectInterval == 0 {
reconnectInterval = defaultReconnectInterval
}
maxReconnectAttempts := c.maxReconnectAttempts
if maxReconnectAttempts == 0 {
maxReconnectAttempts = defaultMaxReconnectAttempts
}
connPoolSize := c.number
for {
if c.IsClosed() {
log.Warnf("client{peer:%s} goroutine exit now.", c.addr)
break
}

num = c.sessionNum()
if max <= num || max < times {
//Exit when the number of connection pools is sufficient or the reconnection times exceeds the connections numbers.
sessionNum = c.sessionNum()
if connPoolSize <= sessionNum || maxReconnectAttempts < reconnectAttempts {
//exit reconnect when the number of connection pools is sufficient or the current reconnection attempts exceeds the max reconnection attempts.
break
}
c.connect()
times++
if times > maxTimes {
maxDuration = int64(maxTimes) * int64(interval)
} else {
maxDuration = int64(times) * int64(interval)
}
<-gxtime.After(time.Duration(maxDuration))
reconnectAttempts++
maxReconnectInterval = int64(math.Min(float64(reconnectAttempts), float64(maxBackOffTimes))) * int64(reconnectInterval)
<-gxtime.After(time.Duration(maxReconnectInterval))
}
}

Expand Down
1 change: 1 addition & 0 deletions transport/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ func TestTCPClient(t *testing.T) {
WithServerAddress(addr.String()),
WithReconnectInterval(5e8),
WithConnectionNumber(1),
WithReconnectAttempts(10),
)
assert.NotNil(t, clt)
assert.True(t, clt.ID() > 0)
Expand Down
17 changes: 13 additions & 4 deletions transport/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ func WithServerTlsConfigBuilder(tlsConfigBuilder TlsConfigBuilder) ServerOption
type ClientOption func(*ClientOptions)

type ClientOptions struct {
addr string
number int
reconnectInterval int // reConnect Interval

addr string
number int
reconnectInterval int // reConnect Interval
maxReconnectAttempts int // max reconnect attempts
// tls
sslEnabled bool
tlsConfigBuilder TlsConfigBuilder
Expand Down Expand Up @@ -168,3 +168,12 @@ func WithClientTlsConfigBuilder(tlsConfigBuilder TlsConfigBuilder) ClientOption
o.tlsConfigBuilder = tlsConfigBuilder
}
}

// WithReconnectAttempts @maxReconnectAttempts is max reconnect attempts.
func WithReconnectAttempts(maxReconnectAttempts int) ClientOption {
return func(o *ClientOptions) {
if 0 < maxReconnectAttempts {
o.maxReconnectAttempts = maxReconnectAttempts
}
}
}
Comment on lines +172 to +179
Copy link
Collaborator

Choose a reason for hiding this comment

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

这段代码并没有被使用,可以先不加或者加注释。否则被人调用之后发现没有效果反而觉得是个问题。

Copy link
Owner

Choose a reason for hiding this comment

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

这段代码并没有被使用,可以先不加或者加注释。否则被人调用之后发现没有效果反而觉得是个问题。

这是给上面提供的接口。要不 @No-SilverBullet 你在代码示例里面调用下?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

11

8 changes: 6 additions & 2 deletions transport/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -860,8 +860,12 @@ func (s *session) stop() {
// let read/Write timeout asap
now := time.Now()
if conn := s.Conn(); conn != nil {
conn.SetReadDeadline(now.Add(s.ReadTimeout()))
conn.SetWriteDeadline(now.Add(s.WriteTimeout()))
if err := conn.SetReadDeadline(now.Add(s.ReadTimeout())); err != nil {
log.Warnf("failed to set read deadline: %+v", err)
}
if err := conn.SetWriteDeadline(now.Add(s.WriteTimeout())); err != nil {
log.Warnf("failed to set write deadline: %+v", err)
}
}
close(s.done)
clt, cltFound := s.GetAttribute(sessionClientKey).(*client)
Expand Down
Loading