[#100] server auto re-binding #109

Merged
alexvanin merged 1 commit from pogpp/frostfs-http-gw:feature/100-rebinding into master 2024-04-08 06:53:29 +00:00
Member

Signed-off-by: Pavel Pogodaev p.pogodaev@yadro.com

Signed-off-by: Pavel Pogodaev <p.pogodaev@yadro.com>
pogpp force-pushed feature/100-rebinding from 027f2a1d2e to 5c0247ac05 2024-03-26 15:28:03 +00:00 Compare
pogpp force-pushed feature/100-rebinding from 5c0247ac05 to 342c2e0be5 2024-03-27 08:28:38 +00:00 Compare
pogpp force-pushed feature/100-rebinding from 342c2e0be5 to 9408cfb704 2024-03-27 08:44:42 +00:00 Compare
pogpp force-pushed feature/100-rebinding from 9408cfb704 to b9bed3109e 2024-03-27 09:29:33 +00:00 Compare
pogpp force-pushed feature/100-rebinding from b9bed3109e to 6741851a30 2024-03-27 10:34:30 +00:00 Compare
pogpp force-pushed feature/100-rebinding from 6741851a30 to fc50d47ce9 2024-03-27 11:31:01 +00:00 Compare
pogpp force-pushed feature/100-rebinding from fc50d47ce9 to f3cf796326 2024-03-27 11:38:54 +00:00 Compare
dkirillov requested review from storage-services-committers 2024-03-27 14:44:07 +00:00
dkirillov requested review from storage-services-developers 2024-03-27 14:44:07 +00:00
pogpp was assigned by dkirillov 2024-03-27 14:44:15 +00:00
dkirillov reviewed 2024-03-27 14:50:09 +00:00
@ -61,0 +60,4 @@
settings *appSettings
servers []Server
unbindServers []ServerInfo
mu sync.RWMutex
Member

Let's separate servers, unbindServers and mu from other fields. It will help see which fields are protected by mutex

Let's separate `servers`, `unbindServers` and `mu` from other fields. It will help see which fields are protected by mutex
dkirillov marked this conversation as resolved
@ -487,6 +497,7 @@ func (a *app) updateSettings() {
a.settings.setBufferMaxSizeForPut(a.cfg.GetUint64(cfgBufferMaxSizeForPut))
a.settings.setNamespaceHeader(a.cfg.GetString(cfgResolveNamespaceHeader))
a.settings.setDefaultNamespaces(a.cfg.GetStringSlice(cfgResolveDefaultNamespaces))
a.settings.setReconnectInterval(a.cfg.GetDuration(cfgReconnectInterval))
Member

In s3-gw we don't update this interval by sighup why we do this here?

In s3-gw we don't update this interval by sighup why we do this here?
Author
Member

In s3-gw we don't update this interval by sighup why we do this here?

Cause updateSettings() for both update and init

> In s3-gw we don't update this interval by sighup why we do this here? Cause updateSettings() for [both](https://git.frostfs.info/TrueCloudLab/frostfs-http-gw/src/branch/master/cmd/http-gw/app.go#L201) update and init
Member

If reconnect_interval shouldn't be updated by sighup we can set it only in init function

If `reconnect_interval` shouldn't be updated by sighup we can set it only in `init` function
dkirillov marked this conversation as resolved
pogpp force-pushed feature/100-rebinding from f3cf796326 to caf494b8a9 2024-04-01 13:03:47 +00:00 Compare
pogpp force-pushed feature/100-rebinding from caf494b8a9 to a6e8a069e1 2024-04-02 14:46:45 +00:00 Compare
Member

Please rebase and make the following changes

diff --git a/cmd/http-gw/app.go b/cmd/http-gw/app.go
index 9de6201..2a20d86 100644
--- a/cmd/http-gw/app.go
+++ b/cmd/http-gw/app.go
@@ -82,6 +82,8 @@ type (
 
 	// appSettings stores reloading parameters, so it has to provide getters and setters which use RWMutex.
 	appSettings struct {
+		reconnectInterval time.Duration
+
 		mu                  sync.RWMutex
 		defaultTimestamp    bool
 		zipCompression      bool
@@ -89,7 +91,6 @@ type (
 		bufferMaxSizeForPut uint64
 		namespaceHeader     string
 		defaultNamespaces   []string
-		reconnectInterval   time.Duration
 	}
 )
 
@@ -204,8 +205,9 @@ func (s *appSettings) setBufferMaxSizeForPut(val uint64) {
 }
 
 func (a *app) initAppSettings() {
-	a.settings = &appSettings{}
-	a.settings.setReconnectInterval(a.cfg.GetDuration(cfgReconnectInterval))
+	a.settings = &appSettings{
+		reconnectInterval: fetchReconnectInterval(a.cfg),
+	}
 	a.updateSettings()
 }
 
@@ -759,12 +761,6 @@ func (s *appSettings) setDefaultNamespaces(namespaces []string) {
 	s.mu.Unlock()
 }
 
-func (s *appSettings) setReconnectInterval(interval time.Duration) {
-	s.mu.Lock()
-	s.reconnectInterval = interval
-	s.mu.Unlock()
-}
-
 func (a *app) scheduleReconnect(ctx context.Context, srv *fasthttp.Server) {
 	go func() {
 		t := time.NewTicker(a.settings.reconnectInterval)
diff --git a/cmd/http-gw/settings.go b/cmd/http-gw/settings.go
index d7f4adb..0d97dcb 100644
--- a/cmd/http-gw/settings.go
+++ b/cmd/http-gw/settings.go
@@ -51,6 +51,8 @@ const (
 
 	defaultNamespaceHeader = "X-Frostfs-Namespace"
 
+	defaultReconnectInterval = time.Minute
+
 	cfgServer      = "server"
 	cfgTLSEnabled  = "tls.enabled"
 	cfgTLSCertFile = "tls.cert_file"
@@ -456,6 +458,15 @@ func getLogLevel(v *viper.Viper) (zapcore.Level, error) {
 	return lvl, nil
 }
 
+func fetchReconnectInterval(cfg *viper.Viper) time.Duration {
+	reconnect := cfg.GetDuration(cfgReconnectInterval)
+	if reconnect <= 0 {
+		reconnect = defaultReconnectInterval
+	}
+
+	return reconnect
+}
+
 func fetchServers(v *viper.Viper, log *zap.Logger) []ServerInfo {
 	var servers []ServerInfo
 	seen := make(map[string]struct{})
diff --git a/docs/gate-configuration.md b/docs/gate-configuration.md
index 04c9454..8e3daad 100644
--- a/docs/gate-configuration.md
+++ b/docs/gate-configuration.md
@@ -85,6 +85,7 @@ reconnect_interval: 1m
 | `rebalance_timer`      | `duration` |               | `60s`          | Interval to check node health.                                                     |
 | `pool_error_threshold` | `uint32`   |               | `100`          | The number of errors on connection after which node is considered as unhealthy.    |
 | `reconnect_interval`   | `duration` | no            | `1m`           | Listeners reconnection interval.                                                   |
+
 # `wallet` section
 
 ```yaml

Please rebase and make the following changes ```diff diff --git a/cmd/http-gw/app.go b/cmd/http-gw/app.go index 9de6201..2a20d86 100644 --- a/cmd/http-gw/app.go +++ b/cmd/http-gw/app.go @@ -82,6 +82,8 @@ type ( // appSettings stores reloading parameters, so it has to provide getters and setters which use RWMutex. appSettings struct { + reconnectInterval time.Duration + mu sync.RWMutex defaultTimestamp bool zipCompression bool @@ -89,7 +91,6 @@ type ( bufferMaxSizeForPut uint64 namespaceHeader string defaultNamespaces []string - reconnectInterval time.Duration } ) @@ -204,8 +205,9 @@ func (s *appSettings) setBufferMaxSizeForPut(val uint64) { } func (a *app) initAppSettings() { - a.settings = &appSettings{} - a.settings.setReconnectInterval(a.cfg.GetDuration(cfgReconnectInterval)) + a.settings = &appSettings{ + reconnectInterval: fetchReconnectInterval(a.cfg), + } a.updateSettings() } @@ -759,12 +761,6 @@ func (s *appSettings) setDefaultNamespaces(namespaces []string) { s.mu.Unlock() } -func (s *appSettings) setReconnectInterval(interval time.Duration) { - s.mu.Lock() - s.reconnectInterval = interval - s.mu.Unlock() -} - func (a *app) scheduleReconnect(ctx context.Context, srv *fasthttp.Server) { go func() { t := time.NewTicker(a.settings.reconnectInterval) diff --git a/cmd/http-gw/settings.go b/cmd/http-gw/settings.go index d7f4adb..0d97dcb 100644 --- a/cmd/http-gw/settings.go +++ b/cmd/http-gw/settings.go @@ -51,6 +51,8 @@ const ( defaultNamespaceHeader = "X-Frostfs-Namespace" + defaultReconnectInterval = time.Minute + cfgServer = "server" cfgTLSEnabled = "tls.enabled" cfgTLSCertFile = "tls.cert_file" @@ -456,6 +458,15 @@ func getLogLevel(v *viper.Viper) (zapcore.Level, error) { return lvl, nil } +func fetchReconnectInterval(cfg *viper.Viper) time.Duration { + reconnect := cfg.GetDuration(cfgReconnectInterval) + if reconnect <= 0 { + reconnect = defaultReconnectInterval + } + + return reconnect +} + func fetchServers(v *viper.Viper, log *zap.Logger) []ServerInfo { var servers []ServerInfo seen := make(map[string]struct{}) diff --git a/docs/gate-configuration.md b/docs/gate-configuration.md index 04c9454..8e3daad 100644 --- a/docs/gate-configuration.md +++ b/docs/gate-configuration.md @@ -85,6 +85,7 @@ reconnect_interval: 1m | `rebalance_timer` | `duration` | | `60s` | Interval to check node health. | | `pool_error_threshold` | `uint32` | | `100` | The number of errors on connection after which node is considered as unhealthy. | | `reconnect_interval` | `duration` | no | `1m` | Listeners reconnection interval. | + # `wallet` section ```yaml ```
alexvanin approved these changes 2024-04-03 13:08:28 +00:00
alexvanin left a comment
Owner

Awesome, please consider some improvements with formatting and less mutex usage from #109 (comment)

Awesome, please consider some improvements with formatting and less mutex usage from https://git.frostfs.info/TrueCloudLab/frostfs-http-gw/pulls/109#issuecomment-36442
pogpp force-pushed feature/100-rebinding from a6e8a069e1 to fb76f69b2a 2024-04-04 11:18:33 +00:00 Compare
pogpp force-pushed feature/100-rebinding from fb76f69b2a to 11965deb41 2024-04-04 11:19:54 +00:00 Compare
dkirillov approved these changes 2024-04-04 11:50:21 +00:00
Member

vulncheck

Vulnerability #1: GO-2024-2687
    HTTP/2 CONTINUATION flood in net/http
  More info: https://pkg.go.dev/vuln/GO-2024-2687
  Module: golang.org/x/net
    Found in: golang.org/x/net@v0.10.0
    Fixed in: golang.org/x/net@v0.23.0

@alexvanin

vulncheck ``` Vulnerability #1: GO-2024-2687 HTTP/2 CONTINUATION flood in net/http More info: https://pkg.go.dev/vuln/GO-2024-2687 Module: golang.org/x/net Found in: golang.org/x/net@v0.10.0 Fixed in: golang.org/x/net@v0.23.0 ``` @alexvanin
pogpp requested review from alexvanin 2024-04-04 18:02:14 +00:00
alexvanin approved these changes 2024-04-08 06:49:40 +00:00
alexvanin merged commit 11965deb41 into master 2024-04-08 06:53:29 +00:00
alexvanin deleted branch feature/100-rebinding 2024-04-08 06:53:30 +00:00
Sign in to join this conversation.
No reviewers
TrueCloudLab/storage-services-developers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-http-gw#109
No description provided.