Skip to content

Commit

Permalink
Avoid establishing separate connection to read Postgres settings
Browse files Browse the repository at this point in the history
In almost all cases we have an existing database connection, so use
that to retrieve the value of Postgres settings.
  • Loading branch information
lfittl committed Jan 8, 2025
1 parent e07d78d commit 620a0e3
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 13 deletions.
2 changes: 1 addition & 1 deletion input/full.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func CollectFull(ctx context.Context, server *state.Server, connection *sql.DB,
logger.PrintError("Error collecting pg_stat_statements: %s", err)
var e *pq.Error
if errors.As(err, &e) && e.Code == "55000" && globalCollectionOpts.TestRun { // object_not_in_prerequisite_state
shared_preload_libraries, _ := postgres.GetPostgresSetting(ctx, "shared_preload_libraries", server, globalCollectionOpts, logger)
shared_preload_libraries, _ := postgres.GetPostgresSetting(ctx, connection, "shared_preload_libraries")
logger.PrintInfo("HINT - Current shared_preload_libraries setting: '%s'. Your Postgres server may need to be restarted for changes to take effect.", shared_preload_libraries)
server.SelfTest.HintCollectionAspect(state.CollectionAspectPgStatStatements, "Current shared_preload_libraries setting: '%s'. Your Postgres server may need to be restarted for changes to take effect.", shared_preload_libraries)
}
Expand Down
11 changes: 2 additions & 9 deletions input/postgres/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

"github.com/guregu/null"
"github.com/pganalyze/collector/state"
"github.com/pganalyze/collector/util"
)

func unpackPostgresInt32Array(input null.String) (result []int32) {
Expand Down Expand Up @@ -72,16 +71,10 @@ SELECT setting
FROM pg_settings
WHERE name = '%s'`

func GetPostgresSetting(ctx context.Context, settingName string, server *state.Server, globalCollectionOpts state.CollectionOpts, prefixedLogger *util.Logger) (string, error) {
func GetPostgresSetting(ctx context.Context, db *sql.DB, settingName string) (string, error) {
var value string

db, err := EstablishConnection(ctx, server, prefixedLogger, globalCollectionOpts, "")
if err != nil {
return "", fmt.Errorf("Could not connect to database to retrieve \"%s\": %s", settingName, err)
}

err = db.QueryRow(QueryMarkerSQL + fmt.Sprintf(settingValueSQL, settingName)).Scan(&value)
db.Close()
err := db.QueryRow(QueryMarkerSQL + fmt.Sprintf(settingValueSQL, settingName)).Scan(&value)
if err != nil {
return "", fmt.Errorf("Could not read \"%s\" setting: %s", settingName, err)
}
Expand Down
2 changes: 1 addition & 1 deletion input/postgres/plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func GetPlans(ctx context.Context, server *state.Server, logger *util.Logger, db
return nil, nil, nil
}

computePlanIdEnabled, err := GetPostgresSetting(ctx, "aurora_compute_plan_id", server, globalCollectionOpts, logger)
computePlanIdEnabled, err := GetPostgresSetting(ctx, db, "aurora_compute_plan_id")
if err != nil {
if globalCollectionOpts.TestRun {
logger.PrintInfo("Function aurora_stat_plans() is not supported because Aurora version is too old. Upgrade to Aurora PostgreSQL version 14.10, 15.5, or later versions to collect query plans and stats.")
Expand Down
2 changes: 1 addition & 1 deletion runner/activity.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func processActivityForServer(ctx context.Context, server *state.Server, globalC
defer connection.Close()
}

trackActivityQuerySize, err := postgres.GetPostgresSetting(ctx, "track_activity_query_size", server, globalCollectionOpts, logger)
trackActivityQuerySize, err := postgres.GetPostgresSetting(ctx, connection, "track_activity_query_size")
if err != nil {
activity.TrackActivityQuerySize = -1
} else {
Expand Down
12 changes: 11 additions & 1 deletion runner/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ func TestLogsForAllServers(ctx context.Context, servers []*state.Server, globalC
continue
}

logLinePrefix, err := postgres.GetPostgresSetting(ctx, "log_line_prefix", server, globalCollectionOpts, prefixedLogger)
logLinePrefix, err := getLogLinePrefix(ctx, server, globalCollectionOpts, prefixedLogger)
if err != nil {
prefixedLogger.PrintError("ERROR - Could not check log_line_prefix for server: %s", err)
hasFailedServers = true
Expand Down Expand Up @@ -426,6 +426,16 @@ func TestLogsForAllServers(ctx context.Context, servers []*state.Server, globalC
return
}

func getLogLinePrefix(ctx context.Context, server *state.Server, globalCollectionOpts state.CollectionOpts, prefixedLogger *util.Logger) (string, error) {
db, err := postgres.EstablishConnection(ctx, server, prefixedLogger, globalCollectionOpts, "")
if err != nil {
return "", fmt.Errorf("Could not connect to database to retrieve log_line_prefix: %s", err)
}
defer db.Close()

return postgres.GetPostgresSetting(ctx, db, "log_line_prefix")
}

func testLocalLogTail(ctx context.Context, wg *sync.WaitGroup, server *state.Server, globalCollectionOpts state.CollectionOpts, logger *util.Logger) bool {
logger.PrintInfo("Testing log collection (local)...")

Expand Down

0 comments on commit 620a0e3

Please sign in to comment.