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

feat: update codis fe / proxy with tls; topom updated #2383

Open
wants to merge 27 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9658b06
update codis proxy to accept tls connection
sprappcom Feb 1, 2024
6ec04e3
tls for proxy works, tls for fe in progress
sprappcom Feb 1, 2024
10b22fc
codis fe tls done
sprappcom Feb 1, 2024
083b737
codis tls fe minor update
sprappcom Feb 1, 2024
4517f87
codis fe minor update
sprappcom Feb 1, 2024
342d893
@chenmin1992 remove my own topom
sprappcom Feb 1, 2024
8ad0b61
optimize code
sprappcom Feb 1, 2024
e233d10
Delete codis/pkg/proxy/.proxy.go.swo
kolinfluence Mar 14, 2024
e99e587
Delete codis/admin/.codis-fe-admin.sh.swo
kolinfluence Mar 14, 2024
c631795
Update store.go
kolinfluence Mar 14, 2024
7b765b7
Update config.go
kolinfluence Mar 14, 2024
6eb8e3a
Update Makefile
kolinfluence Mar 14, 2024
f2cfcee
Update config.go
kolinfluence Mar 14, 2024
b8bda63
Update proxy.toml
kolinfluence Mar 14, 2024
6a8a404
Update proxy.toml
kolinfluence Mar 14, 2024
10038c3
Update codis-fe-admin.sh
kolinfluence Mar 14, 2024
7f78d00
Update dashboard.toml
kolinfluence Mar 14, 2024
8d00a6f
Update topom.go
kolinfluence Mar 14, 2024
e8f24eb
Update main.go
kolinfluence Jul 5, 2024
86afbe7
Update store.go
kolinfluence Jul 5, 2024
1a4e5c4
Update proxy.go
kolinfluence Jul 5, 2024
b9cf0ae
Update topom.go
kolinfluence Jul 5, 2024
a592cc6
Rename cert.pem to cert.pem-dummy
kolinfluence Jul 5, 2024
b7c8ba1
Rename key.pem to key.pem-dummy
kolinfluence Jul 5, 2024
5cfa142
Delete codis/key.pem-dummy
kolinfluence Jul 5, 2024
0d607fa
Delete codis/cert.pem-dummy
kolinfluence Jul 5, 2024
c491c6a
Merge branch 'unstable' into unstable
kolinfluence Jul 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion codis/admin/codis-fe-admin.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ CODIS_FE_DAEMON_FILE=$CODIS_LOG_DIR/codis-fe.out
COORDINATOR_NAME="filesystem"
COORDINATOR_ADDR="/tmp/codis"
CODIS_FE_ADDR="0.0.0.0:9090"
CODIS_FE_TLS=1
CODIS_FE_TLS_KEY="/usr/local/src/pika/codis/key.pem"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment this code

Copy link
Author

Choose a reason for hiding this comment

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

Can you help add the rest?

CODIS_FE_TLS_CERT="/usr/local/src/pika/codis/cert.pem"

echo $CODIS_FE_CONF_FILE

Expand All @@ -36,7 +39,7 @@ start)
fi
fi
nohup "$CODIS_FE_BIN" "--assets-dir=${CODIS_FE_ASSETS_DIR}" "--$COORDINATOR_NAME=$COORDINATOR_ADDR" \
"--log=$CODIS_FE_LOG_FILE" "--pidfile=$CODIS_FE_PID_FILE" "--log-level=INFO" "--listen=$CODIS_FE_ADDR" > "$CODIS_FE_DAEMON_FILE" 2>&1 < /dev/null &
"--tls=$CODIS_FE_TLS" "--tls-key=$CODIS_FE_TLS_KEY" "--tls-cert=$CODIS_FE_TLS_CERT" "--log=$CODIS_FE_LOG_FILE" "--pidfile=$CODIS_FE_PID_FILE" "--log-level=INFO" "--listen=$CODIS_FE_ADDR" > "$CODIS_FE_DAEMON_FILE" 2>&1 < /dev/null &
;;
start-foreground)
$CODIS_FE_BIN "--assets-dir=${CODIS_FE_ASSETS_DIR}" "--$COORDINATOR_NAME=$COORDINATOR_ADDR" \
Expand Down
21 changes: 21 additions & 0 deletions codis/cert.pem
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's better not to provide the public key file. Instead, you can add a generation command in the readme.

Copy link
Author

Choose a reason for hiding this comment

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

@machinly the public key file is definitely a dummy one.
you put "filesystem" as default, surely a dummy version is fine right? ... maybe you can modify with error message generated when there's no cert / key files present after you merge this and delete them. will that do fine?

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
-----BEGIN CERTIFICATE-----
MIIDazCCAlOgAwIBAgIUC/IGKPPBWnAG7MZNPhCUaPmg0oEwDQYJKoZIhvcNAQEL
BQAwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yNDAyMDExNTI0MDBaFw0zNDAx
MjkxNTI0MDBaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw
HwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggEiMA0GCSqGSIb3DQEB
AQUAA4IBDwAwggEKAoIBAQClOgprziC/+nQeHXV5qJA9gdttnx7NV9Cbfc7xox6w
HRCaavLxdKh//E2o6F492U0i+GSqRYWrQKqWix0myLiZwXEL16mkhL7ANLFSnA8C
qsMIJf3u6+rN1OtlIVoKP1zWMFRV0inVCZMC0W6KFeVpuTqewc/FC5w/Zh3shPj1
FJQCOas5SNbpxp9jQQsInur+NeP71r2GcZWD6+H2TCGouMTLFxdL62dbMOGLfm3a
Yq2d+HVhSMmEkuxXEgaIzxJ04toqQntnq4fnRkkFB7CoXG7AQqHIxKb4bjjnD100
qDTTPc1wDEhlye/kUbxztF6dv58AUH1ayg1N7pz3EwazAgMBAAGjUzBRMB0GA1Ud
DgQWBBQtXOfPGePViVYEkHUujTcdtU8RuDAfBgNVHSMEGDAWgBQtXOfPGePViVYE
kHUujTcdtU8RuDAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQA5
HMCZGl8a/fkG8O8XfdNpKvpb7WWINW5pqNf/Ep1wiqx3Y5/2I+NaCn7NhvxkliPU
hF/m3ijJ1eXLVjADMwBuW0vaqS0U29c5k/vWto2xx2Uv7dVffvbGTDUitAygGBTi
9r7A1GnU3PLdA96kUb5RT80tILOSakQYj+KN5QuV4LYmIoUIPlJfn0DOT4D/AoIC
vn/L77gctOQUfzu/l9saN1uwhgBZ06eQv14BJKnUdpmmMDREiP7lAJtQet1dukh6
DvS3QUqf91pkOC6tXMHTAAFQ/op+eJVKUMKfFo1oRkXU1sg1/fD25FKO1xaS6afy
/nr/PY00p+k4s6jw+Fif
-----END CERTIFICATE-----
41 changes: 37 additions & 4 deletions codis/cmd/fe/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package main

import (
"crypto/tls"
"encoding/json"
"fmt"
"io/ioutil"
Expand Down Expand Up @@ -56,7 +57,7 @@ func init() {
func main() {
const usage = `
Usage:
codis-fe [--ncpu=N] [--log=FILE] [--log-level=LEVEL] [--assets-dir=PATH] [--pidfile=FILE] (--dashboard-list=FILE|--zookeeper=ADDR [--zookeeper-auth=USR:PWD]|--etcd=ADDR [--etcd-auth=USR:PWD]|--filesystem=ROOT) --listen=ADDR
codis-fe [--ncpu=N] [--log=FILE] [--log-level=LEVEL] [--assets-dir=PATH] [--pidfile=FILE] (--dashboard-list=FILE|--zookeeper=ADDR [--zookeeper-auth=USR:PWD]|--etcd=ADDR [--etcd-auth=USR:PWD]|--filesystem=ROOT) --listen=ADDR [--tls-cert=FILE] [--tls-key=FILE] [--tls=N]
codis-fe --version

Options:
Expand Down Expand Up @@ -196,10 +197,42 @@ Options:
m.MapTo(r, (*martini.Routes)(nil))
m.Action(r.Handle)

l, err := net.Listen("tcp", listen)
if err != nil {
log.PanicErrorf(err, "listen %s failed", listen)
var l net.Listener

if n, ok := utils.ArgumentInteger(d, "--tls"); ok && n == 1 {

// Load server certificate and key
certFile, keyFile := utils.ArgumentMust(d, "--tls-cert"), utils.ArgumentMust(d, "--tls-key")
if certFile == "" || keyFile == "" {
log.Panic("TLS certificate or key file path must be provided")
}
cert, err := tls.LoadX509KeyPair(certFile, keyFile)
if err != nil {
log.PanicErrorf(err, "failed to load key pair: %s", err)
}

// Create a TLS config
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
}

// Create a TLS listener
l, err = tls.Listen("tcp", listen, tlsConfig)
if err != nil {
log.PanicErrorf(err, "listen %s failed", listen)
}


}else{

l, err = net.Listen("tcp", listen)
if err != nil {
log.PanicErrorf(err, "listen %s failed", listen)
}

Comment on lines +202 to +232
Copy link

Choose a reason for hiding this comment

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

Improve error handling and logging for TLS setup.

Ensure that all potential errors are logged appropriately and provide more context.

	if n, ok := utils.ArgumentInteger(d, "--tls"); ok && n == 1 {
		// Load server certificate and key
		certFile, keyFile := utils.ArgumentMust(d, "--tls-cert"), utils.ArgumentMust(d, "--tls-key")
		if certFile == "" || keyFile == "" {
			log.Panic("TLS certificate or key file path must be provided")
		}
		cert, err := tls.LoadX509KeyPair(certFile, keyFile)
		if err != nil {
			log.PanicErrorf(err, "failed to load key pair: %s", err)
		}

		// Create a TLS config
		tlsConfig := &tls.Config{
			Certificates: []tls.Certificate{cert},
		}

		// Create a TLS listener
		l, err = tls.Listen("tcp", listen, tlsConfig)
		if err != nil {
			log.PanicErrorf(err, "TLS listen on %s failed: %v", listen, err)
		}
		log.Warnf("TLS enabled, listening on %s", listen)
	} else {
		l, err = net.Listen("tcp", listen)
		if err != nil {
			log.PanicErrorf(err, "listen on %s failed: %v", listen, err)
		}
		log.Warnf("Listening on %s without TLS", listen)
	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if n, ok := utils.ArgumentInteger(d, "--tls"); ok && n == 1 {
// Load server certificate and key
certFile, keyFile := utils.ArgumentMust(d, "--tls-cert"), utils.ArgumentMust(d, "--tls-key")
if certFile == "" || keyFile == "" {
log.Panic("TLS certificate or key file path must be provided")
}
cert, err := tls.LoadX509KeyPair(certFile, keyFile)
if err != nil {
log.PanicErrorf(err, "failed to load key pair: %s", err)
}
// Create a TLS config
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
}
// Create a TLS listener
l, err = tls.Listen("tcp", listen, tlsConfig)
if err != nil {
log.PanicErrorf(err, "listen %s failed", listen)
}
}else{
l, err = net.Listen("tcp", listen)
if err != nil {
log.PanicErrorf(err, "listen %s failed", listen)
}
if n, ok := utils.ArgumentInteger(d, "--tls"); ok && n == 1 {
// Load server certificate and key
certFile, keyFile := utils.ArgumentMust(d, "--tls-cert"), utils.ArgumentMust(d, "--tls-key")
if certFile == "" || keyFile == "" {
log.Panic("TLS certificate or key file path must be provided")
}
cert, err := tls.LoadX509KeyPair(certFile, keyFile)
if err != nil {
log.PanicErrorf(err, "failed to load key pair: %s", err)
}
// Create a TLS config
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
}
// Create a TLS listener
l, err = tls.Listen("tcp", listen, tlsConfig)
if err != nil {
log.PanicErrorf(err, "TLS listen on %s failed: %v", listen, err)
}
log.Warnf("TLS enabled, listening on %s", listen)
} else {
l, err = net.Listen("tcp", listen)
if err != nil {
log.PanicErrorf(err, "listen on %s failed: %v", listen, err)
}
log.Warnf("Listening on %s without TLS", listen)
}

}


defer l.Close()

if s, ok := utils.Argument(d, "--pidfile"); ok {
Expand Down
3 changes: 3 additions & 0 deletions codis/config/proxy.toml
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not modify the default configuration

Copy link
Author

Choose a reason for hiding this comment

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

can you only pull what is needed?

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ admin_addr = "0.0.0.0:11080"

# Set bind address for proxy, proto_type can be "tcp", "tcp4", "tcp6", "unix" or "unixpacket".
proto_type = "tcp4"
#proxy_tls = true
#proxy_tls_cert = "/usr/local/src/pika/codis/cert.pem"
#proxy_tls_key = "/usr/local/src/pika/codis/key.pem"
proxy_addr = "0.0.0.0:19000"

# Set jodis address & session timeout
Expand Down
28 changes: 28 additions & 0 deletions codis/key.pem
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's better not to provide the private key file. Instead, you can add a generation command in the readme.

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQClOgprziC/+nQe
HXV5qJA9gdttnx7NV9Cbfc7xox6wHRCaavLxdKh//E2o6F492U0i+GSqRYWrQKqW
ix0myLiZwXEL16mkhL7ANLFSnA8CqsMIJf3u6+rN1OtlIVoKP1zWMFRV0inVCZMC
0W6KFeVpuTqewc/FC5w/Zh3shPj1FJQCOas5SNbpxp9jQQsInur+NeP71r2GcZWD
6+H2TCGouMTLFxdL62dbMOGLfm3aYq2d+HVhSMmEkuxXEgaIzxJ04toqQntnq4fn
RkkFB7CoXG7AQqHIxKb4bjjnD100qDTTPc1wDEhlye/kUbxztF6dv58AUH1ayg1N
7pz3EwazAgMBAAECggEANou0MAXTGv2wQtbf1uN2hs+tZIGg1hJ0/DgqzuvWcDaQ
KiI/hMXJ8MKW0rmvjwSDahWNiga8i54kEuHnJEwkYSIS02GFkBoArQxYM9jwPaWp
EDIWpD7++6ecdHzvsC1d1RoMZW6lv75S6Z3BO9XhsCblus5p7HeaQ/XO0CvP0nxJ
SxT1Mj6U0dkiGVPWmmsKuWacYUkbYLeOh0s+T298vm5sRaECvewqwGSQYO8EJVaW
c/sFfJPPwONRGK3+Ggt/Puq6z+6i8baiXnMYHBhHhxdHN8fgRjGW+J+fYddOmd90
Sfr+OeECnqXKvIQdnH3WOOzvSF8oP0/Si7dEu8mF1QKBgQC1erZifnvnKPTn2Y7t
lfdbP9tbuBo4e6PXKf7oy3gEMb7DBoXlLrytnyKS2CnNfw3y+Cptk6sHvCRlf00h
1WSQqLf7+6LvwqZunhLD+lIqbBeYb6FnjU32HTyTA37nojO9G7F+4uUktWtAp5TV
zp04k/lK4jFC63/GWc9/iHauZQKBgQDpEtYgGcmtCv2+9pKHfKIqC3Ma2HzV9EsW
VTgVjRZWxSIxeblPI8GqCEPB0Wjn/GcZ83MIpGH4RlPkZVM1z8DkWEBn9OxQysCl
t8UEIysAParbwdXQ4bLeVmRO8eEXhSSRQgyIPGHQ+oNECbZftnkMIMZqNIP5Tz3C
mg4zfjDjNwKBgCDWwrgR8TPEGoT1vkJJt8fgRz5SkxQTc3NU2xae8um3YrCBtqrh
CS1VtXji+rV/vzNvKqZHaVRt/BwNrBRqO9ddYGWNhE9kZp9vpS+nVUTt6FsiIA/P
5wKZCcQEhus9U6VtpHG0dwhsd6rsaCdESvjY9exR/93/JVDsIXsPY4JtAoGAXCjg
lkzcHBpuf2YMRpqXnLxuRT05D3jLgAcuaAcjwCeu/5U8GvjuiQc/k5H0Q1bghbhn
0hD91Cgq3/LY1jxR73id7v0/vCqPxndF5f7p7+xydPbOTwwPqC7M9xJZiO1SqkVN
o0kM/I8x/GNCw9HpR0xmaHRfguM8bohv+Rh6yD0CgYAfEVhTk+fXshrMeH9qdEwH
AzfQaQtPpQISgoTADT2ZDkZSSDOuVtMK0L/ZvAAeL+jbQh3RJayARfp2o8BnD12U
YIaBq9OC8JUZbG71e93zY/zv4HmZdK/RymmwM2jv9UxTnQPpnHRuVcYF0HZpZSWJ
Xf12eCJQmM/HGdynEvZtcw==
-----END PRIVATE KEY-----
3 changes: 3 additions & 0 deletions codis/pkg/models/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ type Proxy struct {

ProtoType string `json:"proto_type"`
ProxyAddr string `json:"proxy_addr"`
ProxyTLS bool `json:"proxy_tls"`
ProxyTLSCert string `json:"proxy_tls_cert"`
ProxyTLSKey string `json:"proxy_tls_key"`

JodisPath string `json:"jodis_path,omitempty"`

Expand Down
19 changes: 19 additions & 0 deletions codis/pkg/models/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,25 @@ func (s *Store) Release() error {
return s.client.Delete(s.LockPath())
}

func (s *Store) ReleaseByToken(token string) error {
b, err := s.client.Read(s.LockPath(),false)
if err != nil {
return err
}

if b != nil {
t, err := Decode(b);
if err != nil {
return err
}
if t.Token == token {
return s.Release()
}
}

return nil
}
Copy link

@coderabbitai coderabbitai bot Jul 5, 2024

Choose a reason for hiding this comment

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

Reduce nesting and improve readability.

The nested if statements can be reduced to improve readability.

func (s *Store) ReleaseByToken(token string) error {
	b, err := s.client.Read(s.LockPath(), false)
	if err != nil {
		return err
	}
	if b == nil {
		return nil
	}

	t, err := Decode(b)
	if err != nil {
		return err
	}
	if t.Token == token {
		return s.Release()
	}
	return nil
}

Copy link
Author

Choose a reason for hiding this comment

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

@coderabbitai no. nesting is fine here.

Copy link

Choose a reason for hiding this comment

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

@kolinfluence Understood, I'll respect your decision to keep the nesting as is. If you need any further assistance, feel free to ask.


func (s *Store) LoadTopom(must bool) (*Topom, error) {
return LoadTopom(s.client, s.product, must)
}
Expand Down
8 changes: 8 additions & 0 deletions codis/pkg/models/topom.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,11 @@ type Topom struct {
func (t *Topom) Encode() []byte {
return jsonEncode(t)
}

func Decode(b []byte) (*Topom, error) {
s := &Topom{}
if err := jsonDecode(s, b); err != nil {
return nil,err
}
return s,nil
Copy link

Choose a reason for hiding this comment

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

Fix formatting issues.

There are minor formatting issues in the Decode function. Ensure there is a space after the comma in the return statements.

-    return nil,err
+    return nil, err

-    return s,nil
+    return s, nil
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func Decode(b []byte) (*Topom, error) {
s := &Topom{}
if err := jsonDecode(s, b); err != nil {
return nil,err
}
return s,nil
func Decode(b []byte) (*Topom, error) {
s := &Topom{}
if err := jsonDecode(s, b); err != nil {
return nil, err
}
return s, nil

}
6 changes: 6 additions & 0 deletions codis/pkg/proxy/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ admin_addr = "0.0.0.0:11080"

# Set bind address for proxy, proto_type can be "tcp", "tcp4", "tcp6", "unix" or "unixpacket".
proto_type = "tcp4"
#proxy_tls = true
#proxy_tls_cert = "/usr/local/src/pika/codis/cert.pem"
#proxy_tls_key = "/usr/local/src/pika/codis/key.pem"
proxy_addr = "0.0.0.0:19000"

# Set jodis address & session timeout
Expand Down Expand Up @@ -166,6 +169,9 @@ type Config struct {

ProxyDataCenter string `toml:"proxy_datacenter" json:"proxy_datacenter"`
ProxyMaxClients int `toml:"proxy_max_clients" json:"proxy_max_clients"`
ProxyTLS bool `toml:"proxy_tls" json:"proxy_tls"`
ProxyTLSCert string `toml:"proxy_tls_cert" json:"proxy_tls_cert"`
ProxyTLSKey string `toml:"proxy_tls_key" json:"proxy_tls_key"`
ProxyMaxOffheapBytes bytesize.Int64 `toml:"proxy_max_offheap_size" json:"proxy_max_offheap_size"`
ProxyHeapPlaceholder bytesize.Int64 `toml:"proxy_heap_placeholder" json:"proxy_heap_placeholder"`

Expand Down
50 changes: 43 additions & 7 deletions codis/pkg/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package proxy

import (
"crypto/tls"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -103,19 +104,54 @@ func New(config *Config) (*Proxy, error) {

func (p *Proxy) setup(config *Config) error {
proto := config.ProtoType
if l, err := net.Listen(proto, config.ProxyAddr); err != nil {
return errors.Trace(err)

var l net.Listener
var err error

if config.ProxyTLS {
log.Printf("TLS configuration: cert = %s, key = %s", config.ProxyTLSCert, config.ProxyTLSKey)

// Load the TLS certificate
cert, err := tls.LoadX509KeyPair(config.ProxyTLSCert, config.ProxyTLSKey)
if err != nil {
return errors.Trace(err)
}

// Set up TLS configuration to use TLS 1.2 or higher
tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: tls.VersionTLS12,
}

// Create a TLS listener
l, err = tls.Listen(proto, config.ProxyAddr, tlsConfig)
if err != nil {
return errors.Trace(err)
}
} else {
p.lproxy = l
log.Printf("Setting up a non-TLS listener")

x, err := utils.ReplaceUnspecifiedIP(proto, l.Addr().String(), config.HostProxy)
// Create a non-TLS listener
l, err = net.Listen(proto, config.ProxyAddr)
if err != nil {
return err
return errors.Trace(err)
}
p.model.ProtoType = proto
p.model.ProxyAddr = x
}

// Common setup for both TLS and non-TLS
p.lproxy = l

// Replace unspecified IP address with the specific host proxy address
x, err := utils.ReplaceUnspecifiedIP(proto, l.Addr().String(), config.HostProxy)
if err != nil {
return err
}

// Update proxy model
p.model.ProtoType = proto
p.model.ProxyAddr = x


proto = "tcp"
if l, err := net.Listen(proto, config.AdminAddr); err != nil {
return errors.Trace(err)
Expand Down
8 changes: 5 additions & 3 deletions codis/pkg/topom/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ migration_async_numkeys = 500
migration_timeout = "30s"

# Set configs for redis sentinel.
sentinel_check_server_state_interval = "5s"
sentinel_check_master_failover_interval = "1s"
sentinel_master_dead_check_times = 5
sentinel_check_server_state_interval = "10s"
sentinel_check_master_failover_interval = "2s"
sentinel_master_dead_check_times = 10
sentinel_check_offline_server_interval = "2s"
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

Ensure new configuration setting is used.

The new configuration setting sentinel_check_offline_server_interval is defined but not used anywhere in the codebase. Please ensure that this setting is utilized where necessary or remove it if it is not required.

  • codis/pkg/topom/config.go: Definition found, but no usage detected.
Analysis chain

Ensure new configuration setting is used.

The new configuration setting sentinel_check_offline_server_interval should be used in the code. Verify its usage or remove it if unnecessary.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new configuration setting `sentinel_check_offline_server_interval`.

# Test: Search for the usage of the new configuration setting. Expect: At least one occurrence.
rg --type go 'sentinel_check_offline_server_interval'

Length of output: 300


Script:

#!/bin/bash
# Description: Verify the usage of the configuration setting `SentinelCheckOfflineServerInterval`.

# Test: Search for the usage of the configuration setting. Expect: At least one occurrence.
rg --type go 'SentinelCheckOfflineServerInterval'

Length of output: 224

sentinel_client_timeout = "10s"
sentinel_quorum = 2
sentinel_parallel_syncs = 1
Expand Down Expand Up @@ -86,6 +87,7 @@ type Config struct {
SentinelCheckServerStateInterval timesize.Duration `toml:"sentinel_check_server_state_interval" json:"sentinel_client_timeout"`
SentinelCheckMasterFailoverInterval timesize.Duration `toml:"sentinel_check_master_failover_interval" json:"sentinel_check_master_failover_interval"`
SentinelMasterDeadCheckTimes int8 `toml:"sentinel_master_dead_check_times" json:"sentinel_master_dead_check_times"`
SentinelCheckOfflineServerInterval timesize.Duration `toml:"sentinel_check_offline_server_interval" json:"sentinel_check_offline_server_interval"`
kolinfluence marked this conversation as resolved.
Show resolved Hide resolved
SentinelClientTimeout timesize.Duration `toml:"sentinel_client_timeout" json:"sentinel_client_timeout"`
SentinelQuorum int `toml:"sentinel_quorum" json:"sentinel_quorum"`
SentinelParallelSyncs int `toml:"sentinel_parallel_syncs" json:"sentinel_parallel_syncs"`
Expand Down
3 changes: 2 additions & 1 deletion codis/pkg/topom/topom.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ func (s *Topom) Close() error {
defer s.store.Close()

if s.online {
if err := s.store.Release(); err != nil {
err := s.store.ReleaseByToken(s.model.Token);
if err != nil {
Comment on lines +172 to +173
Copy link

Choose a reason for hiding this comment

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

Improve error handling in Close method.

Ensure that the error is logged and handled appropriately.

	if s.online {
		err := s.store.ReleaseByToken(s.model.Token)
		if err != nil {
			log.ErrorErrorf(err, "store: release lock of %s failed", s.config.ProductName)
			return errors.Errorf("store: release lock of %s failed", s.config.ProductName)
		}
	}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
err := s.store.ReleaseByToken(s.model.Token);
if err != nil {
if s.online {
err := s.store.ReleaseByToken(s.model.Token)
if err != nil {
log.ErrorErrorf(err, "store: release lock of %s failed", s.config.ProductName)
return errors.Errorf("store: release lock of %s failed", s.config.ProductName)
}
}

log.ErrorErrorf(err, "store: release lock of %s failed", s.config.ProductName)
return errors.Errorf("store: release lock of %s failed", s.config.ProductName)
}
Expand Down
Loading