Skip to content

Commit

Permalink
prevent vtctld from creating tons of S3 connections (#15296)
Browse files Browse the repository at this point in the history
Signed-off-by: Renan Rangel <[email protected]>
Signed-off-by: 'Renan Rangel' <[email protected]>
  • Loading branch information
vitess-bot[bot] committed Mar 4, 2024
1 parent 283b5c2 commit 2c59593
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
25 changes: 17 additions & 8 deletions go/vt/mysqlctl/s3backupstorage/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,10 +281,21 @@ func (s3ServerSideEncryption *S3ServerSideEncryption) reset() {

// S3BackupStorage implements the backupstorage.BackupStorage interface.
type S3BackupStorage struct {
_client *s3.S3
mu sync.Mutex
s3SSE S3ServerSideEncryption
params backupstorage.Params
_client *s3.S3
mu sync.Mutex
s3SSE S3ServerSideEncryption
params backupstorage.Params
transport *http.Transport
}

func newS3BackupStorage() *S3BackupStorage {
// This initialises a new transport based off http.DefaultTransport the first time and returns the same
// transport on subsequent calls so connections can be reused as part of the same transport.
tlsClientConf := &tls.Config{InsecureSkipVerify: tlsSkipVerifyCert}
transport := http.DefaultTransport.(*http.Transport).Clone()
transport.TLSClientConfig = tlsClientConf

return &S3BackupStorage{params: backupstorage.NoParams(), transport: transport}
}

// ListBackups is part of the backupstorage.BackupStorage interface.
Expand Down Expand Up @@ -445,9 +456,7 @@ func (bs *S3BackupStorage) client() (*s3.S3, error) {
if bs._client == nil {
logLevel := getLogLevel()

tlsClientConf := &tls.Config{InsecureSkipVerify: tlsSkipVerifyCert}
httpTransport := &http.Transport{TLSClientConfig: tlsClientConf}
httpClient := &http.Client{Transport: httpTransport}
httpClient := &http.Client{Transport: bs.transport}

Check warning on line 459 in go/vt/mysqlctl/s3backupstorage/s3.go

View check run for this annotation

Codecov / codecov/patch

go/vt/mysqlctl/s3backupstorage/s3.go#L459

Added line #L459 was not covered by tests

session, err := session.NewSession()
if err != nil {
Expand Down Expand Up @@ -497,7 +506,7 @@ func objName(parts ...string) *string {
}

func init() {
backupstorage.BackupStorageMap["s3"] = &S3BackupStorage{params: backupstorage.NoParams()}
backupstorage.BackupStorageMap["s3"] = newS3BackupStorage()

logNameMap = logNameToLogLevel{
"LogOff": aws.LogOff,
Expand Down
10 changes: 10 additions & 0 deletions go/vt/mysqlctl/s3backupstorage/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,3 +276,13 @@ func TestSSECustomerFileBase64Key(t *testing.T) {
assert.Nil(t, sseData.customerKey, "customerKey expected to be nil")
assert.Nil(t, sseData.customerMd5, "customerMd5 expected to be nil")
}

func TestNewS3Transport(t *testing.T) {
s3 := newS3BackupStorage()

// checking some of the values are present in the returned transport and match the http.DefaultTransport.
assert.Equal(t, http.DefaultTransport.(*http.Transport).IdleConnTimeout, s3.transport.IdleConnTimeout)
assert.Equal(t, http.DefaultTransport.(*http.Transport).MaxIdleConns, s3.transport.MaxIdleConns)
assert.NotNil(t, s3.transport.DialContext)
assert.NotNil(t, s3.transport.Proxy)
}

0 comments on commit 2c59593

Please sign in to comment.