diff --git a/postgresql/config.go b/postgresql/config.go index c2f1410c..eac08403 100644 --- a/postgresql/config.go +++ b/postgresql/config.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" "sync" + "time" "unicode" "github.com/blang/semver" @@ -179,6 +180,9 @@ type Config struct { Timeout int ConnectTimeoutSec int MaxConns int + MaxIdleConns int + ConnMaxIdleTime time.Duration + ConnMaxLifetime time.Duration ExpectedVersion semver.Version SSLClientCert *ClientCertificateConfig SSLRootCertPath string @@ -308,8 +312,10 @@ func (c *Client) Connect() (*DBConnection, error) { // We don't want to retain connection // So when we connect on a specific database which might be managed by terraform, // we don't keep opened connection in case of the db has to be dropped in the plan. - db.SetMaxIdleConns(0) + db.SetMaxIdleConns(c.config.MaxIdleConns) db.SetMaxOpenConns(c.config.MaxConns) + db.SetConnMaxIdleTime(c.config.ConnMaxIdleTime) + db.SetConnMaxIdleTime(c.config.ConnMaxLifetime) defaultVersion, _ := semver.Parse(defaultExpectedPostgreSQLVersion) version := &c.config.ExpectedVersion diff --git a/postgresql/provider.go b/postgresql/provider.go index 8bc7546d..9d9474fb 100644 --- a/postgresql/provider.go +++ b/postgresql/provider.go @@ -3,9 +3,11 @@ package postgresql import ( "context" "fmt" + "os" + "time" + "github.com/aws/aws-sdk-go-v2/credentials" "github.com/aws/aws-sdk-go-v2/service/sts" - "os" "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" @@ -21,8 +23,12 @@ import ( ) const ( - defaultProviderMaxOpenConnections = 20 - defaultExpectedPostgreSQLVersion = "9.0.0" + defaultProviderMaxOpenConnections = 20 // sql.DB Default + defaultProviderMaxIdleConnections = 0 + // Keep connection lifetimes short by default, to prevent blocking 'destroy' actions on database instance resources. + defaultProviderConnectionMaxIdleTime = time.Second * 5 + defaultProviderConnectionMaxLifetime = time.Second * 10 + defaultExpectedPostgreSQLVersion = "9.0.0" ) // Provider returns a terraform.ResourceProvider. @@ -192,6 +198,35 @@ func Provider() *schema.Provider { Description: "Maximum number of connections to establish to the database. Zero means unlimited.", ValidateFunc: validation.IntAtLeast(-1), }, + "max_idle_connections": { + Type: schema.TypeInt, + Optional: true, + Default: defaultProviderMaxIdleConnections, + Description: "Maximum number of idle connections to hold for the database. " + + "NOTE: Idle connections that aren't cleaned-up can cause problems if a database is destroyed in " + + "the same plan. The default is 0 to prevent that issue.", + ValidateFunc: validation.IntAtLeast(-1), + }, + "connection_max_idle_time": { + Type: schema.TypeString, + Optional: true, + Default: defaultProviderConnectionMaxIdleTime.String(), + Description: "Duration to hold idle connections to the database. Setting to 0 will hold connections " + "" + + "forever. " + + "NOTE: Idle connections that aren't cleaned-up can cause problems if a database is destroyed in " + + "the same plan. This should be > 0 to prevent that.", + ValidateFunc: validateParsableDuration, + }, + "connection_max_lifetime": { + Type: schema.TypeString, + Optional: true, + Default: defaultProviderConnectionMaxLifetime.String(), + Description: "Maximum lifetime of any connections to the database. Setting to 0 will hold " + + "connections forever. " + + "NOTE: Idle connections that aren't cleaned-up can cause problems if a database is destroyed in " + + "the same plan. This should be > 0 to prevent that.", + ValidateFunc: validateParsableDuration, + }, "expected_version": { Type: schema.TypeString, Optional: true, @@ -236,6 +271,13 @@ func validateExpectedVersion(v interface{}, key string) (warnings []string, erro return } +func validateParsableDuration(v any, _ string) (warnings []string, errors []error) { + if _, err := time.ParseDuration(v.(string)); err != nil { + errors = append(errors, fmt.Errorf("invalid duration (%q): %w", v.(string), err)) + } + return +} + func getRDSAuthToken(region string, profile string, role string, username string, host string, port int) (string, error) { endpoint := fmt.Sprintf("%s:%d", host, port) @@ -367,6 +409,10 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) { password = d.Get("password").(string) } + // Safe to ignore the error, since this is already checked in validation + connMaxIdleTime, _ := time.ParseDuration(d.Get("connection_max_idle_time").(string)) + connMaxLifetime, _ := time.ParseDuration(d.Get("connection_max_lifetime").(string)) + config := Config{ Scheme: d.Get("scheme").(string), Host: host, @@ -379,6 +425,9 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) { ApplicationName: "Terraform provider", ConnectTimeoutSec: d.Get("connect_timeout").(int), MaxConns: d.Get("max_connections").(int), + MaxIdleConns: d.Get("max_idle_connections").(int), + ConnMaxIdleTime: connMaxIdleTime, + ConnMaxLifetime: connMaxLifetime, ExpectedVersion: version, SSLRootCertPath: d.Get("sslrootcert").(string), GCPIAMImpersonateServiceAccount: d.Get("gcp_iam_impersonate_service_account").(string), diff --git a/website/docs/index.html.markdown b/website/docs/index.html.markdown index bc0a1b03..91d39e1d 100644 --- a/website/docs/index.html.markdown +++ b/website/docs/index.html.markdown @@ -172,6 +172,15 @@ The following arguments are supported: default is `180s`. Zero or not specified means wait indefinitely. * `max_connections` - (Optional) Set the maximum number of open connections to the database. The default is `20`. Zero means unlimited open connections. +* `max_idle_connections` - (Optional) Maximum number of idle connections to hold for the database. NOTE: Idle + connections that aren't cleaned-up can cause problems if a database is destroyed in the same plan. The default is 0 to + prevent that issue. +* `connection_max_idle_time` - (Optional) Duration to hold idle connections to the database. Setting to 0 will hold + connections forever. NOTE: Idle connections that aren't cleaned-up can cause problems if a database is destroyed in + the same plan. This should be > 0 to prevent that. +* `connection_max_lifetime` - (Optional) Maximum lifetime of any connections to the database. Setting to 0 will hold + connections forever. NOTE: Idle connections that aren't cleaned-up can cause problems if a database is destroyed in + the same plan. This should be > 0 to prevent that. * `expected_version` - (Optional) Specify a hint to Terraform regarding the expected version that the provider will be talking with. This is a required hint in order for Terraform to talk with an ancient version of PostgreSQL.