Address review feedback

pganalyze · Dec 27, 2024 · 7014175 · 7014175
1 parent d0f4359
commit 7014175
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 171 deletions.
diff --git a/input/postgres/explain_analyze.go b/input/postgres/explain_analyze.go
@@ -11,7 +11,7 @@ import (
 	"github.com/pganalyze/collector/util"
 )
 
-func runExplainAnalyzeWithHelper(ctx context.Context, db *sql.DB, query string, parameters []null.String, parameterTypes []string, analyzeFlags []string, marker string) (explainOutput string, err error) {
+func runExplainAnalyze(ctx context.Context, db *sql.DB, query string, parameters []null.String, parameterTypes []string, analyzeFlags []string, marker string) (explainOutput string, err error) {
 	tx, err := db.BeginTx(ctx, &sql.TxOptions{ReadOnly: true})
 	if err != nil {
 		return "", err
@@ -38,41 +38,29 @@ func validateQuery(query string) error {
 	return nil
 }
 
-func RunExplainAnalyzeForQueryRun(ctx context.Context, db *sql.DB, query string, parameters []null.String, parameterTypes []string, marker string) (_result string, _err error) {
-	err := validateQuery(query)
+func RunExplainAnalyzeForQueryRun(ctx context.Context, db *sql.DB, query string, parameters []null.String, parameterTypes []string, marker string) (result string, err error) {
+	err = validateQuery(query)
 	if err != nil {
-		return "", err
+		return
 	}
 
 	// Warm up caches without collecting timing info (slightly faster)
-	_, err = runExplainAnalyzeWithHelper(ctx, db, query, parameters, parameterTypes, []string{"ANALYZE", "TIMING OFF"}, marker)
+	_, err = runExplainAnalyze(ctx, db, query, parameters, parameterTypes, []string{"ANALYZE", "TIMING OFF"}, marker)
 
-	if err != nil && !strings.Contains(err.Error(), "statement timeout") {
-		// Run again if it was a timeout error, to make sure we got the caches warmed up all the way
-		_, err = runExplainAnalyzeWithHelper(ctx, db, query, parameters, parameterTypes, []string{"ANALYZE", "TIMING OFF"}, marker)
-		if ctx.Err() != nil {
-			return "", err
-		}
+	// Run again if it was a timeout error, to make sure we got the caches warmed up all the way
+	if err != nil && strings.Contains(err.Error(), "statement timeout") {
+		_, err = runExplainAnalyze(ctx, db, query, parameters, parameterTypes, []string{"ANALYZE", "TIMING OFF"}, marker)
 
 		// If it timed out again, capture a non-ANALYZE EXPLAIN instead
 		if err != nil && strings.Contains(err.Error(), "statement timeout") {
-			explainOutput, err := runExplainAnalyzeWithHelper(ctx, db, query, parameters, parameterTypes, []string{}, marker)
-			if err != nil {
-				return "", err
-			}
-			return explainOutput, nil
+			return runExplainAnalyze(ctx, db, query, parameters, parameterTypes, []string{}, marker)
 		} else if err != nil {
-			return "", err
+			return
 		}
 	} else if err != nil {
-		return "", err
+		return
 	}
 
 	// Run EXPLAIN ANALYZE once more to get a warm cache result (this is the one we return)
-	explainOutput, err := runExplainAnalyzeWithHelper(ctx, db, query, parameters, parameterTypes, []string{"ANALYZE", "BUFFERS"}, marker)
-	if err != nil {
-		return "", err
-	}
-
-	return explainOutput, nil
+	return runExplainAnalyze(ctx, db, query, parameters, parameterTypes, []string{"ANALYZE", "BUFFERS"}, marker)
 }
diff --git a/output/pganalyze_collector/server_message.pb.go b/output/pganalyze_collector/server_message.pb.go
diff --git a/protobuf/server_message.proto b/protobuf/server_message.proto
@@ -39,18 +39,13 @@ message ServerMessage {
     bool pause = 1;
   }
 
-  message QueryRunPostgresSetting {
-    string name = 1;
-    string value = 2;
-  }
-
   message QueryRun {
     int64 id = 1;
     QueryRunType type = 2;
     string database_name = 3;
     string query_text = 4;
     repeated NullString query_parameters = 5;
     repeated string query_parameter_types = 6;
-    repeated QueryRunPostgresSetting postgres_settings = 7;
+    map<string, string> postgres_settings = 7;
   }
 }