TudbuT/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 4

Set self-adjusting deadline for connection writing (#16068)

Browse source 
* Set self-adjusting deadline for connection writing

In #16055 it appears that the simple 5s deadline doesn't work for large
file writes. Now we can't - or at least shouldn't just set no deadline
as go will happily let these connections block indefinitely. However,
what seems reasonable is to set some minimum rate we expect for writing.

This PR suggests the following algorithm:

* Every write has a minimum timeout of 5s (adjustable at compile time.)
* If there has been a previous write - then consider its previous
deadline, add half of the minimum timeout + 2s per kb about to written.
* If that new deadline is after the minimum timeout use that.

Fix #16055

* Linearly increase timeout

* Make PerWriteTimeout, PerWritePerKbTimeouts configurable

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: 6543 <6543@obermui.de>
This commit is contained in:
zeripath 2021-06-10 22:25:25 +01:00 committed by GitHub
parent 86e2789960
commit daa5a23548
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 90 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
custom/conf/app.example.ini
View file

@ -51,6 +51,12 @@ RUN_MODE = ; prod
;REDIRECT_OTHER_PORT = false ;REDIRECT_OTHER_PORT = false
;PORT_TO_REDIRECT = 80 ;PORT_TO_REDIRECT = 80
;; ;;
;; Timeout for any write to the connection. (Set to 0 to disable all timeouts.)
;PER_WRITE_TIMEOUT = 30s
;;
;; Timeout per Kb written to connections.
;PER_WRITE_PER_KB_TIMEOUT = 30s
;;
;; Permission for unix socket ;; Permission for unix socket
;UNIX_SOCKET_PERMISSION = 666 ;UNIX_SOCKET_PERMISSION = 666
;; ;;
@ -144,6 +150,14 @@ RUN_MODE = ; prod
;; Enable exposure of SSH clone URL to anonymous visitors, default is false ;; Enable exposure of SSH clone URL to anonymous visitors, default is false
;SSH_EXPOSE_ANONYMOUS = false ;SSH_EXPOSE_ANONYMOUS = false
;; ;;
;; Timeout for any write to ssh connections. (Set to 0 to disable all timeouts.)
;; Will default to the PER_WRITE_TIMEOUT.
;SSH_PER_WRITE_TIMEOUT = 30s
;;
;; Timeout per Kb written to ssh connections.
;; Will default to the PER_WRITE_PER_KB_TIMEOUT.
;SSH_PER_WRITE_PER_KB_TIMEOUT = 30s
;;
;; Indicate whether to check minimum key size with corresponding type ;; Indicate whether to check minimum key size with corresponding type
;MINIMUM_KEY_SIZE_CHECK = false ;MINIMUM_KEY_SIZE_CHECK = false
;; ;;
@ -1145,8 +1159,8 @@ PATH =
;; ;;
;; When ISSUE_INDEXER_QUEUE_TYPE is levelqueue, this will be the path where the queue will be saved. ;; When ISSUE_INDEXER_QUEUE_TYPE is levelqueue, this will be the path where the queue will be saved.
;; This can be overridden by `ISSUE_INDEXER_QUEUE_CONN_STR`. ;; This can be overridden by `ISSUE_INDEXER_QUEUE_CONN_STR`.
;; default is queues/common ;; default is queues/common
;ISSUE_INDEXER_QUEUE_DIR = queues/common ;ISSUE_INDEXER_QUEUE_DIR = queues/common
;; ;;
;; When `ISSUE_INDEXER_QUEUE_TYPE` is `redis`, this will store the redis connection string. ;; When `ISSUE_INDEXER_QUEUE_TYPE` is `redis`, this will store the redis connection string.
;; When `ISSUE_INDEXER_QUEUE_TYPE` is `levelqueue`, this is a directory or additional options of ;; When `ISSUE_INDEXER_QUEUE_TYPE` is `levelqueue`, this is a directory or additional options of
@ -1201,7 +1215,7 @@ PATH =
;; default to persistable-channel ;; default to persistable-channel
;TYPE = persistable-channel ;TYPE = persistable-channel
;; ;;
;; data-dir for storing persistable queues and level queues, individual queues will default to `queues/common` meaning the queue is shared. ;; data-dir for storing persistable queues and level queues, individual queues will default to `queues/common` meaning the queue is shared.
;DATADIR = queues/ ;DATADIR = queues/
;; ;;
;; Default queue length before a channel queue will block ;; Default queue length before a channel queue will block

10
docs/content/doc/advanced/config-cheat-sheet.en-us.md
View file

@ -251,6 +251,9 @@ The following configuration set `Content-Type: application/vnd.android.package-a
most cases you do not need to change the default value. Alter it only if most cases you do not need to change the default value. Alter it only if
your SSH server node is not the same as HTTP node. Do not set this variable your SSH server node is not the same as HTTP node. Do not set this variable
if `PROTOCOL` is set to `unix`. if `PROTOCOL` is set to `unix`.
- `PER_WRITE_TIMEOUT`: **30s**: Timeout for any write to the connection. (Set to 0 to
disable all timeouts.)
- `PER_WRITE_PER_KB_TIMEOUT`: **10s**: Timeout per Kb written to connections.
- `DISABLE_SSH`: **false**: Disable SSH feature when it's not available. - `DISABLE_SSH`: **false**: Disable SSH feature when it's not available.
- `START_SSH_SERVER`: **false**: When enabled, use the built-in SSH server. - `START_SSH_SERVER`: **false**: When enabled, use the built-in SSH server.
@ -274,6 +277,9 @@ The following configuration set `Content-Type: application/vnd.android.package-a
- `SSH_KEY_TEST_PATH`: **/tmp**: Directory to create temporary files in when testing public keys using ssh-keygen, default is the system temporary directory. - `SSH_KEY_TEST_PATH`: **/tmp**: Directory to create temporary files in when testing public keys using ssh-keygen, default is the system temporary directory.
- `SSH_KEYGEN_PATH`: **ssh-keygen**: Path to ssh-keygen, default is 'ssh-keygen' which means the shell is responsible for finding out which one to call. - `SSH_KEYGEN_PATH`: **ssh-keygen**: Path to ssh-keygen, default is 'ssh-keygen' which means the shell is responsible for finding out which one to call.
- `SSH_EXPOSE_ANONYMOUS`: **false**: Enable exposure of SSH clone URL to anonymous visitors, default is false. - `SSH_EXPOSE_ANONYMOUS`: **false**: Enable exposure of SSH clone URL to anonymous visitors, default is false.
- `SSH_PER_WRITE_TIMEOUT`: **30s**: Timeout for any write to the SSH connections. (Set to
0 to disable all timeouts.)
- `SSH_PER_WRITE_PER_KB_TIMEOUT`: **10s**: Timeout per Kb written to SSH connections.
- `MINIMUM_KEY_SIZE_CHECK`: **true**: Indicate whether to check minimum key size with corresponding type. - `MINIMUM_KEY_SIZE_CHECK`: **true**: Indicate whether to check minimum key size with corresponding type.
- `OFFLINE_MODE`: **false**: Disables use of CDN for static files and Gravatar for profile pictures. - `OFFLINE_MODE`: **false**: Disables use of CDN for static files and Gravatar for profile pictures.
@ -350,7 +356,7 @@ relation to port exhaustion.
- `ISSUE_INDEXER_PATH`: **indexers/issues.bleve**: Index file used for issue search; available when ISSUE_INDEXER_TYPE is bleve and elasticsearch. - `ISSUE_INDEXER_PATH`: **indexers/issues.bleve**: Index file used for issue search; available when ISSUE_INDEXER_TYPE is bleve and elasticsearch.
- The next 4 configuration values are deprecated and should be set in `queue.issue_indexer` however are kept for backwards compatibility: - The next 4 configuration values are deprecated and should be set in `queue.issue_indexer` however are kept for backwards compatibility:
- `ISSUE_INDEXER_QUEUE_TYPE`: **levelqueue**: Issue indexer queue, currently supports:`channel`, `levelqueue`, `redis`. - `ISSUE_INDEXER_QUEUE_TYPE`: **levelqueue**: Issue indexer queue, currently supports:`channel`, `levelqueue`, `redis`.
- `ISSUE_INDEXER_QUEUE_DIR`: **queues/common**: When `ISSUE_INDEXER_QUEUE_TYPE` is `levelqueue`, this will be the path where the queue will be saved. (Previously this was `indexers/issues.queue`.) - `ISSUE_INDEXER_QUEUE_DIR`: **queues/common**: When `ISSUE_INDEXER_QUEUE_TYPE` is `levelqueue`, this will be the path where the queue will be saved. (Previously this was `indexers/issues.queue`.)
- `ISSUE_INDEXER_QUEUE_CONN_STR`: **addrs=127.0.0.1:6379 db=0**: When `ISSUE_INDEXER_QUEUE_TYPE` is `redis`, this will store the redis connection string. When `ISSUE_INDEXER_QUEUE_TYPE` is `levelqueue`, this is a directory or additional options of the form `leveldb://path/to/db?option=value&....`, and overrides `ISSUE_INDEXER_QUEUE_DIR`. - `ISSUE_INDEXER_QUEUE_CONN_STR`: **addrs=127.0.0.1:6379 db=0**: When `ISSUE_INDEXER_QUEUE_TYPE` is `redis`, this will store the redis connection string. When `ISSUE_INDEXER_QUEUE_TYPE` is `levelqueue`, this is a directory or additional options of the form `leveldb://path/to/db?option=value&....`, and overrides `ISSUE_INDEXER_QUEUE_DIR`.
- `ISSUE_INDEXER_QUEUE_BATCH_NUMBER`: **20**: Batch queue number. - `ISSUE_INDEXER_QUEUE_BATCH_NUMBER`: **20**: Batch queue number.
@ -370,7 +376,7 @@ relation to port exhaustion.
## Queue (`queue` and `queue.*`) ## Queue (`queue` and `queue.*`)
- `TYPE`: **persistable-channel**: General queue type, currently support: `persistable-channel` (uses a LevelDB internally), `channel`, `level`, `redis`, `dummy` - `TYPE`: **persistable-channel**: General queue type, currently support: `persistable-channel` (uses a LevelDB internally), `channel`, `level`, `redis`, `dummy`
- `DATADIR`: **queues/**: Base DataDir for storing persistent and level queues. `DATADIR` for individual queues can be set in `queue.name` sections but will default to `DATADIR/`**`common`**. (Previously each queue would default to `DATADIR/`**`name`**.) - `DATADIR`: **queues/**: Base DataDir for storing persistent and level queues. `DATADIR` for individual queues can be set in `queue.name` sections but will default to `DATADIR/`**`common`**. (Previously each queue would default to `DATADIR/`**`name`**.)
- `LENGTH`: **20**: Maximal queue size before channel queues block - `LENGTH`: **20**: Maximal queue size before channel queues block
- `BATCH_LENGTH`: **20**: Batch data before passing to the handler - `BATCH_LENGTH`: **20**: Batch data before passing to the handler
- `CONN_STR`: **redis://127.0.0.1:6379/0**: Connection string for the redis queue type. Options can be set using query params. Similarly LevelDB options can also be set using: **leveldb://relative/path?option=value** or **leveldb:///absolute/path?option=value**, and will override `DATADIR` - `CONN_STR`: **redis://127.0.0.1:6379/0**: Connection string for the redis queue type. Options can be set using query params. Similarly LevelDB options can also be set using: **leveldb://relative/path?option=value** or **leveldb:///absolute/path?option=value**, and will override `DATADIR`

64
modules/graceful/server.go
View file

@ -17,6 +17,7 @@ import (
"time" "time"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
) )
var ( var (
@ -26,11 +27,12 @@ var (
DefaultWriteTimeOut time.Duration DefaultWriteTimeOut time.Duration
// DefaultMaxHeaderBytes default max header bytes // DefaultMaxHeaderBytes default max header bytes
DefaultMaxHeaderBytes int DefaultMaxHeaderBytes int
// PerWriteWriteTimeout timeout for writes
PerWriteWriteTimeout = 30 * time.Second
// PerWriteWriteTimeoutKbTime is a timeout taking account of how much there is to be written
PerWriteWriteTimeoutKbTime = 10 * time.Second
) )
// PerWriteWriteTimeout timeout for writes
const PerWriteWriteTimeout = 5 * time.Second
func init() { func init() {
DefaultMaxHeaderBytes = 0 // use http.DefaultMaxHeaderBytes - which currently is 1 << 20 (1MB) DefaultMaxHeaderBytes = 0 // use http.DefaultMaxHeaderBytes - which currently is 1 << 20 (1MB)
} }
@ -40,14 +42,16 @@ type ServeFunction = func(net.Listener) error
// Server represents our graceful server // Server represents our graceful server
type Server struct { type Server struct {
network string network string
address string address string
listener net.Listener listener net.Listener
wg sync.WaitGroup wg sync.WaitGroup
state state state state
lock *sync.RWMutex lock *sync.RWMutex
BeforeBegin func(network, address string) BeforeBegin func(network, address string)
OnShutdown func() OnShutdown func()
PerWriteTimeout time.Duration
PerWritePerKbTimeout time.Duration
} }
// NewServer creates a server on network at provided address // NewServer creates a server on network at provided address
@ -58,11 +62,13 @@ func NewServer(network, address, name string) *Server {
log.Info("Starting new %s server: %s:%s on PID: %d", name, network, address, os.Getpid()) log.Info("Starting new %s server: %s:%s on PID: %d", name, network, address, os.Getpid())
} }
srv := &Server{ srv := &Server{
wg: sync.WaitGroup{}, wg: sync.WaitGroup{},
state: stateInit, state: stateInit,
lock: &sync.RWMutex{}, lock: &sync.RWMutex{},
network: network, network: network,
address: address, address: address,
PerWriteTimeout: setting.PerWriteTimeout,
PerWritePerKbTimeout: setting.PerWritePerKbTimeout,
} }
srv.BeforeBegin = func(network, addr string) { srv.BeforeBegin = func(network, addr string) {
@ -224,9 +230,11 @@ func (wl *wrappedListener) Accept() (net.Conn, error) {
closed := int32(0) closed := int32(0)
c = wrappedConn{ c = wrappedConn{
Conn: c, Conn: c,
server: wl.server, server: wl.server,
closed: &closed, closed: &closed,
perWriteTimeout: wl.server.PerWriteTimeout,
perWritePerKbTimeout: wl.server.PerWritePerKbTimeout,
} }
wl.server.wg.Add(1) wl.server.wg.Add(1)
@ -249,13 +257,23 @@ func (wl *wrappedListener) File() (*os.File, error) {
type wrappedConn struct { type wrappedConn struct {
net.Conn net.Conn
server *Server server *Server
closed *int32 closed *int32
deadline time.Time
perWriteTimeout time.Duration
perWritePerKbTimeout time.Duration
} }
func (w wrappedConn) Write(p []byte) (n int, err error) { func (w wrappedConn) Write(p []byte) (n int, err error) {
if PerWriteWriteTimeout > 0 { if w.perWriteTimeout > 0 {
_ = w.Conn.SetWriteDeadline(time.Now().Add(PerWriteWriteTimeout)) minTimeout := time.Duration(len(p)/1024) * w.perWritePerKbTimeout
minDeadline := time.Now().Add(minTimeout).Add(w.perWriteTimeout)
w.deadline = w.deadline.Add(minTimeout)
if minDeadline.After(w.deadline) {
w.deadline = minDeadline
}
_ = w.Conn.SetWriteDeadline(w.deadline)
} }
return w.Conn.Write(p) return w.Conn.Write(p)
} }

32
modules/setting/setting.go
View file

@ -117,6 +117,8 @@ var (
GracefulRestartable bool GracefulRestartable bool
GracefulHammerTime time.Duration GracefulHammerTime time.Duration
StartupTimeout time.Duration StartupTimeout time.Duration
PerWriteTimeout = 30 * time.Second
PerWritePerKbTimeout = 10 * time.Second
StaticURLPrefix string StaticURLPrefix string
AbsoluteAssetURL string AbsoluteAssetURL string
@ -147,18 +149,22 @@ var (
TrustedUserCAKeys []string `ini:"SSH_TRUSTED_USER_CA_KEYS"` TrustedUserCAKeys []string `ini:"SSH_TRUSTED_USER_CA_KEYS"`
TrustedUserCAKeysFile string `ini:"SSH_TRUSTED_USER_CA_KEYS_FILENAME"` TrustedUserCAKeysFile string `ini:"SSH_TRUSTED_USER_CA_KEYS_FILENAME"`
TrustedUserCAKeysParsed []gossh.PublicKey `ini:"-"` TrustedUserCAKeysParsed []gossh.PublicKey `ini:"-"`
PerWriteTimeout time.Duration `ini:"SSH_PER_WRITE_TIMEOUT"`
PerWritePerKbTimeout time.Duration `ini:"SSH_PER_WRITE_PER_KB_TIMEOUT"`
}{ }{
Disabled: false, Disabled: false,
StartBuiltinServer: false, StartBuiltinServer: false,
Domain: "", Domain: "",
Port: 22, Port: 22,
ServerCiphers: []string{"aes128-ctr", "aes192-ctr", "aes256-ctr", "aes128-gcm@openssh.com", "arcfour256", "arcfour128"}, ServerCiphers: []string{"aes128-ctr", "aes192-ctr", "aes256-ctr", "aes128-gcm@openssh.com", "arcfour256", "arcfour128"},
ServerKeyExchanges: []string{"diffie-hellman-group1-sha1", "diffie-hellman-group14-sha1", "ecdh-sha2-nistp256", "ecdh-sha2-nistp384", "ecdh-sha2-nistp521", "curve25519-sha256@libssh.org"}, ServerKeyExchanges: []string{"diffie-hellman-group1-sha1", "diffie-hellman-group14-sha1", "ecdh-sha2-nistp256", "ecdh-sha2-nistp384", "ecdh-sha2-nistp521", "curve25519-sha256@libssh.org"},
ServerMACs: []string{"hmac-sha2-256-etm@openssh.com", "hmac-sha2-256", "hmac-sha1", "hmac-sha1-96"}, ServerMACs: []string{"hmac-sha2-256-etm@openssh.com", "hmac-sha2-256", "hmac-sha1", "hmac-sha1-96"},
KeygenPath: "ssh-keygen", KeygenPath: "ssh-keygen",
MinimumKeySizeCheck: true, MinimumKeySizeCheck: true,
MinimumKeySizes: map[string]int{"ed25519": 256, "ed25519-sk": 256, "ecdsa": 256, "ecdsa-sk": 256, "rsa": 2048}, MinimumKeySizes: map[string]int{"ed25519": 256, "ed25519-sk": 256, "ecdsa": 256, "ecdsa-sk": 256, "rsa": 2048},
ServerHostKeys: []string{"ssh/gitea.rsa", "ssh/gogs.rsa"}, ServerHostKeys: []string{"ssh/gitea.rsa", "ssh/gogs.rsa"},
PerWriteTimeout: PerWriteTimeout,
PerWritePerKbTimeout: PerWritePerKbTimeout,
} }
// Security settings // Security settings
@ -612,6 +618,8 @@ func NewContext() {
GracefulRestartable = sec.Key("ALLOW_GRACEFUL_RESTARTS").MustBool(true) GracefulRestartable = sec.Key("ALLOW_GRACEFUL_RESTARTS").MustBool(true)
GracefulHammerTime = sec.Key("GRACEFUL_HAMMER_TIME").MustDuration(60 * time.Second) GracefulHammerTime = sec.Key("GRACEFUL_HAMMER_TIME").MustDuration(60 * time.Second)
StartupTimeout = sec.Key("STARTUP_TIMEOUT").MustDuration(0 * time.Second) StartupTimeout = sec.Key("STARTUP_TIMEOUT").MustDuration(0 * time.Second)
PerWriteTimeout = sec.Key("PER_WRITE_TIMEOUT").MustDuration(PerWriteTimeout)
PerWritePerKbTimeout = sec.Key("PER_WRITE_PER_KB_TIMEOUT").MustDuration(PerWritePerKbTimeout)
defaultAppURL := string(Protocol) + "://" + Domain defaultAppURL := string(Protocol) + "://" + Domain
if (Protocol == HTTP && HTTPPort != "80") || (Protocol == HTTPS && HTTPPort != "443") { if (Protocol == HTTP && HTTPPort != "80") || (Protocol == HTTPS && HTTPPort != "443") {
@ -777,6 +785,8 @@ func NewContext() {
} }
SSH.ExposeAnonymous = sec.Key("SSH_EXPOSE_ANONYMOUS").MustBool(false) SSH.ExposeAnonymous = sec.Key("SSH_EXPOSE_ANONYMOUS").MustBool(false)
SSH.PerWriteTimeout = sec.Key("SSH_PER_WRITE_TIMEOUT").MustDuration(PerWriteTimeout)
SSH.PerWritePerKbTimeout = sec.Key("SSH_PER_WRITE_PER_KB_TIMEOUT").MustDuration(PerWritePerKbTimeout)
if err = Cfg.Section("oauth2").MapTo(&OAuth2); err != nil { if err = Cfg.Section("oauth2").MapTo(&OAuth2); err != nil {
log.Fatal("Failed to OAuth2 settings: %v", err) log.Fatal("Failed to OAuth2 settings: %v", err)

3
modules/ssh/ssh_graceful.go
View file

@ -7,12 +7,15 @@ package ssh
import ( import (
"code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/graceful"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"github.com/gliderlabs/ssh" "github.com/gliderlabs/ssh"
) )
func listen(server *ssh.Server) { func listen(server *ssh.Server) {
gracefulServer := graceful.NewServer("tcp", server.Addr, "SSH") gracefulServer := graceful.NewServer("tcp", server.Addr, "SSH")
gracefulServer.PerWriteTimeout = setting.SSH.PerWriteTimeout
gracefulServer.PerWritePerKbTimeout = setting.SSH.PerWritePerKbTimeout
err := gracefulServer.ListenAndServe(server.Serve) err := gracefulServer.ListenAndServe(server.Serve)
if err != nil { if err != nil {