Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions state/oracledatabase/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Comment thread
nelson-parente marked this conversation as resolved.
Outdated
- 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"
61 changes: 54 additions & 7 deletions state/oracledatabase/oracledatabaseaccess.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Comment thread
nelson-parente marked this conversation as resolved.
Outdated

// 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.
Expand Down Expand Up @@ -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.
Comment thread
nelson-parente marked this conversation as resolved.
Outdated
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)
Expand Down Expand Up @@ -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)
}
Comment thread
nelson-parente marked this conversation as resolved.

Expand Down
132 changes: 132 additions & 0 deletions state/oracledatabase/oracledatabaseaccess_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
}
Loading