From 98b72d9ec47890f1f3c4321258193dc1548b9201 Mon Sep 17 00:00:00 2001 From: Nelson Parente Date: Wed, 17 Jun 2026 09:36:28 +0200 Subject: [PATCH 1/3] feat: add connection pool options to Oracle state store Expose four database/sql connection-pool tunables as component metadata so operators can tune the Oracle state store without forking the component: maxOpenConns -> db.SetMaxOpenConns(int) maxIdleConns -> db.SetMaxIdleConns(int) connMaxLifetime -> db.SetConnMaxLifetime(time.Duration) connMaxIdleTime -> db.SetConnMaxIdleTime(time.Duration) Each setter is called only when the user provides the option (non-zero value), preserving Go's built-in database/sql defaults when the field is omitted. The pattern and field names mirror the MySQL bindings component (bindings/mysql). Fixes #4276 Signed-off-by: Nelson Parente --- state/oracledatabase/metadata.yaml | 36 +++++ state/oracledatabase/oracledatabaseaccess.go | 61 +++++++- .../oracledatabaseaccess_test.go | 132 ++++++++++++++++++ 3 files changed, 222 insertions(+), 7 deletions(-) diff --git a/state/oracledatabase/metadata.yaml b/state/oracledatabase/metadata.yaml index b8e2764010..e4b62dc501 100644 --- a/state/oracledatabase/metadata.yaml +++ b/state/oracledatabase/metadata.yaml @@ -40,3 +40,39 @@ metadata: are clamped with a warning log. example: "100" default: "1000" + - name: maxOpenConns + type: number + required: false + description: | + Maximum number of open connections to the Oracle database. + Maps to db.SetMaxOpenConns in database/sql. + A value of 0 or omitting this field leaves the Go default (unlimited). + example: "10" + default: "0" + - name: maxIdleConns + type: number + required: false + description: | + Maximum number of idle connections in the connection pool. + Maps to db.SetMaxIdleConns in database/sql. + A value of 0 or omitting this field leaves the Go default (2). + example: "5" + default: "0" + - name: connMaxLifetime + type: duration + required: false + description: | + Maximum amount of time a connection may be reused before it is closed. + Maps to db.SetConnMaxLifetime in database/sql. + A value of 0 or omitting this field leaves the Go default (unlimited). + example: "30m" + default: "0s" + - name: connMaxIdleTime + type: duration + required: false + description: | + Maximum amount of time a connection may be idle before it is closed. + Maps to db.SetConnMaxIdleTime in database/sql. + A value of 0 or omitting this field leaves the Go default (unlimited). + example: "10m" + default: "0s" diff --git a/state/oracledatabase/oracledatabaseaccess.go b/state/oracledatabase/oracledatabaseaccess.go index 79b993b8a8..2b341952a7 100644 --- a/state/oracledatabase/oracledatabaseaccess.go +++ b/state/oracledatabase/oracledatabaseaccess.go @@ -61,16 +61,39 @@ type oracleDatabaseAccess struct { } type oracleDatabaseMetadata struct { - ConnectionString string `json:"connectionString"` - OracleWalletLocation string `json:"oracleWalletLocation"` - TableName string `json:"tableName"` + ConnectionString string `json:"connectionString" mapstructure:"connectionString"` + OracleWalletLocation string `json:"oracleWalletLocation" mapstructure:"oracleWalletLocation"` + TableName string `json:"tableName" mapstructure:"tableName"` // BulkGetChunkSize controls the maximum number of keys included in each // internal SQL query issued by BulkGet. When the number of requested // keys exceeds this value, BulkGet issues multiple chunked queries // sequentially (not in parallel) and merges the results. Values less // than or equal to 0 default to 1000; values above 1000 are clamped. // See normalizeBulkGetChunkSize. - BulkGetChunkSize int `json:"bulkGetChunkSize"` + BulkGetChunkSize int `json:"bulkGetChunkSize" mapstructure:"bulkGetChunkSize"` + + // Connection pool options — map to database/sql setters. + // A zero value means "not provided"; the corresponding setter is skipped + // to avoid overriding Go's built-in defaults unintentionally. + + // MaxOpenConns is the maximum number of open connections to the database. + // Maps to db.SetMaxOpenConns. A value of 0 leaves the Go default (unlimited). + MaxOpenConns int `mapstructure:"maxOpenConns"` + + // MaxIdleConns is the maximum number of connections in the idle connection pool. + // Maps to db.SetMaxIdleConns. A value of 0 leaves the Go default (2). + // Note: database/sql treats 0 as "use default (2)" — there is no way to + // express "set to 0" via this field without a separate "was explicitly set" + // sentinel. This is a known limitation of the current metadata schema. + MaxIdleConns int `mapstructure:"maxIdleConns"` + + // ConnMaxLifetime is the maximum amount of time a connection may be reused. + // Maps to db.SetConnMaxLifetime. A value of 0 leaves the Go default (unlimited). + ConnMaxLifetime time.Duration `mapstructure:"connMaxLifetime"` + + // ConnMaxIdleTime is the maximum amount of time a connection may be idle. + // Maps to db.SetConnMaxIdleTime. A value of 0 leaves the Go default (unlimited). + ConnMaxIdleTime time.Duration `mapstructure:"connMaxIdleTime"` } // newOracleDatabaseAccess creates a new instance of oracleDatabaseAccess. @@ -107,6 +130,24 @@ func normalizeBulkGetChunkSize(log logger.Logger, configured int) int { return configured } +// applyConnectionPool applies non-zero connection pool settings to db. +// Zero values are skipped so Go's built-in defaults are preserved. +// Called by Init after sql.Open and before PingContext. +func applyConnectionPool(db *sql.DB, m *oracleDatabaseMetadata) { + if m.MaxOpenConns > 0 { + db.SetMaxOpenConns(m.MaxOpenConns) + } + if m.MaxIdleConns > 0 { + db.SetMaxIdleConns(m.MaxIdleConns) + } + if m.ConnMaxLifetime > 0 { + db.SetConnMaxLifetime(m.ConnMaxLifetime) + } + if m.ConnMaxIdleTime > 0 { + db.SetConnMaxIdleTime(m.ConnMaxIdleTime) + } +} + // Init sets up OracleDatabase connection and ensures that the state table exists. func (o *oracleDatabaseAccess) Init(ctx context.Context, metadata state.Metadata) error { meta, err := parseMetadata(metadata.Properties) @@ -134,13 +175,19 @@ func (o *oracleDatabaseAccess) Init(ctx context.Context, metadata state.Metadata return err } - o.db = db + // Apply connection pool settings only when the user explicitly provided them, + // so we don't override Go's built-in defaults with zero values unintentionally. + applyConnectionPool(db, &meta) - err = db.PingContext(ctx) - if err != nil { + // Ping before assigning o.db so that a failed ping does not leave an + // unreachable *sql.DB handle in the struct (DB handle leak fix). + if err = db.PingContext(ctx); err != nil { + _ = db.Close() return err } + o.db = db + return o.ensureStateTable(o.metadata.TableName) } diff --git a/state/oracledatabase/oracledatabaseaccess_test.go b/state/oracledatabase/oracledatabaseaccess_test.go index 175424337e..d8d5adf58b 100644 --- a/state/oracledatabase/oracledatabaseaccess_test.go +++ b/state/oracledatabase/oracledatabaseaccess_test.go @@ -904,3 +904,135 @@ func TestBulkGetChunking_SingleChunkFastPath(t *testing.T) { // Only one query issued require.NoError(t, mock.ExpectationsWereMet()) } + +// TestParseMetadataConnectionPool verifies that connection pool fields are parsed +// correctly from the metadata properties map. +func TestParseMetadataConnectionPool(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + props map[string]string + wantMaxOpen int + wantMaxIdle int + wantMaxLifetime time.Duration + wantMaxIdleTime time.Duration + }{ + { + name: "all pool options set", + props: map[string]string{ + "connectionString": "oracle://user:pass@localhost:1521/svc", + "maxOpenConns": "20", + "maxIdleConns": "5", + "connMaxLifetime": "30m", + "connMaxIdleTime": "10m", + }, + wantMaxOpen: 20, + wantMaxIdle: 5, + wantMaxLifetime: 30 * time.Minute, + wantMaxIdleTime: 10 * time.Minute, + }, + { + name: "no pool options — zero values, Go defaults apply", + props: map[string]string{ + "connectionString": "oracle://user:pass@localhost:1521/svc", + }, + wantMaxOpen: 0, + wantMaxIdle: 0, + wantMaxLifetime: 0, + wantMaxIdleTime: 0, + }, + { + name: "only maxOpenConns set", + props: map[string]string{ + "connectionString": "oracle://user:pass@localhost:1521/svc", + "maxOpenConns": "100", + }, + wantMaxOpen: 100, + wantMaxIdle: 0, + wantMaxLifetime: 0, + wantMaxIdleTime: 0, + }, + { + name: "duration using seconds shorthand", + props: map[string]string{ + "connectionString": "oracle://user:pass@localhost:1521/svc", + "connMaxLifetime": "90s", + "connMaxIdleTime": "45s", + }, + wantMaxOpen: 0, + wantMaxIdle: 0, + wantMaxLifetime: 90 * time.Second, + wantMaxIdleTime: 45 * time.Second, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + meta, err := parseMetadata(tt.props) + require.NoError(t, err) + assert.Equal(t, tt.wantMaxOpen, meta.MaxOpenConns, "MaxOpenConns") + assert.Equal(t, tt.wantMaxIdle, meta.MaxIdleConns, "MaxIdleConns") + assert.Equal(t, tt.wantMaxLifetime, meta.ConnMaxLifetime, "ConnMaxLifetime") + assert.Equal(t, tt.wantMaxIdleTime, meta.ConnMaxIdleTime, "ConnMaxIdleTime") + }) + } +} + +// TestApplyConnectionPool_MaxOpenConns verifies that applyConnectionPool sets +// MaxOpenConns on the *sql.DB when provided, and that the value is reflected in +// db.Stats().MaxOpenConnections (the only pool field exposed by database/sql). +func TestApplyConnectionPool_MaxOpenConns(t *testing.T) { + t.Parallel() + + db, _, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + + meta := oracleDatabaseMetadata{ + ConnectionString: "oracle://user:pass@localhost:1521/svc", + TableName: defaultTableName, + BulkGetChunkSize: defaultBulkGetChunkSize, + MaxOpenConns: 25, + MaxIdleConns: 8, + ConnMaxLifetime: 15 * time.Minute, + ConnMaxIdleTime: 5 * time.Minute, + } + + // Exercise production helper — not inline guard logic. + applyConnectionPool(db, &meta) + + stats := db.Stats() + assert.Equal(t, 25, stats.MaxOpenConnections, + "MaxOpenConns should be reflected in db.Stats().MaxOpenConnections") + // database/sql does not expose MaxIdleConns, ConnMaxLifetime, or + // ConnMaxIdleTime in DBStats; coverage for those fields comes from the + // parsing tests (TestParseMetadataConnectionPool) which verify decoding, + // and from the helper running without error on a sqlmock DB (no panic). +} + +// TestApplyConnectionPool_ZeroValuesSkipped verifies that zero-value pool +// options are skipped by applyConnectionPool, leaving Go's built-in defaults. +func TestApplyConnectionPool_ZeroValuesSkipped(t *testing.T) { + t.Parallel() + + db, _, err := sqlmock.New() + require.NoError(t, err) + defer db.Close() + + meta := oracleDatabaseMetadata{ + ConnectionString: "oracle://user:pass@localhost:1521/svc", + TableName: defaultTableName, + BulkGetChunkSize: defaultBulkGetChunkSize, + // all pool options are zero — applyConnectionPool must skip setters + } + + // Exercise production helper — must not panic, must preserve defaults. + applyConnectionPool(db, &meta) + + // MaxOpenConnections of 0 means unlimited (Go default). + stats := db.Stats() + assert.Equal(t, 0, stats.MaxOpenConnections, + "zero MaxOpenConns should leave Go default (unlimited)") +} From f0edcd7c9a0e611ecb6f7f86df1a650fdfaaf9f4 Mon Sep 17 00:00:00 2001 From: Nelson Parente Date: Thu, 18 Jun 2026 21:30:43 +0100 Subject: [PATCH 2/3] fix(oracle): close DB on ensureStateTable failure; correct maxIdleConns docs (Copilot review) Signed-off-by: Nelson Parente --- state/oracledatabase/metadata.yaml | 6 ++++-- state/oracledatabase/oracledatabaseaccess.go | 15 ++++++++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/state/oracledatabase/metadata.yaml b/state/oracledatabase/metadata.yaml index e4b62dc501..436af49b2b 100644 --- a/state/oracledatabase/metadata.yaml +++ b/state/oracledatabase/metadata.yaml @@ -55,9 +55,11 @@ metadata: description: | Maximum number of idle connections in the connection pool. Maps to db.SetMaxIdleConns in database/sql. - A value of 0 or omitting this field leaves the Go default (2). + When set to 0 or omitted, the setter is skipped and the Go database/sql + default of 2 idle connections applies. To change the idle pool size, + set this to a value greater than 0. example: "5" - default: "0" + default: "2" - name: connMaxLifetime type: duration required: false diff --git a/state/oracledatabase/oracledatabaseaccess.go b/state/oracledatabase/oracledatabaseaccess.go index 2b341952a7..25afb272eb 100644 --- a/state/oracledatabase/oracledatabaseaccess.go +++ b/state/oracledatabase/oracledatabaseaccess.go @@ -81,10 +81,10 @@ type oracleDatabaseMetadata struct { MaxOpenConns int `mapstructure:"maxOpenConns"` // MaxIdleConns is the maximum number of connections in the idle connection pool. - // Maps to db.SetMaxIdleConns. A value of 0 leaves the Go default (2). - // Note: database/sql treats 0 as "use default (2)" — there is no way to - // express "set to 0" via this field without a separate "was explicitly set" - // sentinel. This is a known limitation of the current metadata schema. + // Maps to db.SetMaxIdleConns. When the value is 0 (not provided), the setter + // is skipped entirely so Go's database/sql default of 2 idle connections applies. + // Note: calling SetMaxIdleConns(0) would DISABLE idle connections, which is why + // we skip the call rather than passing the zero value through. MaxIdleConns int `mapstructure:"maxIdleConns"` // ConnMaxLifetime is the maximum amount of time a connection may be reused. @@ -188,7 +188,12 @@ func (o *oracleDatabaseAccess) Init(ctx context.Context, metadata state.Metadata o.db = db - return o.ensureStateTable(o.metadata.TableName) + if err = o.ensureStateTable(o.metadata.TableName); err != nil { + _ = o.db.Close() + o.db = nil + return err + } + return nil } func parseConnectionString(meta oracleDatabaseMetadata) (string, error) { From 53bb8e2357a693e333ea99a4d9abaeb8ea8d941a Mon Sep 17 00:00:00 2001 From: Nelson Parente Date: Fri, 19 Jun 2026 00:45:20 +0100 Subject: [PATCH 3/3] docs(oracle): clarify applyConnectionPool applies positive values only (Copilot review) Signed-off-by: Nelson Parente --- state/oracledatabase/oracledatabaseaccess.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/state/oracledatabase/oracledatabaseaccess.go b/state/oracledatabase/oracledatabaseaccess.go index 25afb272eb..14afa7846c 100644 --- a/state/oracledatabase/oracledatabaseaccess.go +++ b/state/oracledatabase/oracledatabaseaccess.go @@ -130,8 +130,8 @@ func normalizeBulkGetChunkSize(log logger.Logger, configured int) int { return configured } -// applyConnectionPool applies non-zero connection pool settings to db. -// Zero values are skipped so Go's built-in defaults are preserved. +// applyConnectionPool applies positive connection pool settings to db. +// Values <= 0 (zero or negative) are skipped so Go's built-in defaults are preserved. // Called by Init after sql.Open and before PingContext. func applyConnectionPool(db *sql.DB, m *oracleDatabaseMetadata) { if m.MaxOpenConns > 0 {