various small tweaks and fixes (#90)

* bugfix: handle queries with no plans Signed-off-by: Andres Taylor <[email protected]> * tweak flag descriptions Signed-off-by: Andres Taylor <[email protected]> * bugfix: csv flag handling was broken Signed-off-by: Andres Taylor <[email protected]> --------- Signed-off-by: Andres Taylor <[email protected]>
vitessio · Jan 9, 2025 · 0d5adc5 · 0d5adc5
1 parent c243d67
commit 0d5adc5
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 29 deletions.
diff --git a/go/cmd/flag_config.go b/go/cmd/flag_config.go
@@ -89,5 +89,8 @@ func csvFlagsToConfig(cmd *cobra.Command, flags csvFlags) data.CSVConfig {
 	if cmd.Flags().Changed("csv-timestamp-field") {
 		c.TimestampField = &flags.timestampField
 	}
+	if cmd.Flags().Changed("csv-header") {
+		c.Header = flags.header
+	}
 	return c
 }
diff --git a/go/cmd/keys.go b/go/cmd/keys.go
@@ -32,10 +32,8 @@ func keysCmd() *cobra.Command {
 		Short:   "Runs vexplain keys on all queries of the test file",
 		Example: "vt keys file.test",
 		Args:    cobra.ExactArgs(1),
-		PreRun: func(cmd *cobra.Command, _ []string) {
-			csvConfig = csvFlagsToConfig(cmd, *flags)
-		},
 		RunE: func(c *cobra.Command, args []string) error {
+			csvConfig = csvFlagsToConfig(c, *flags)
 			cfg := keys.Config{
 				FileName: args[0],
 			}

diff --git a/go/cmd/planalyze.go b/go/cmd/planalyze.go
@@ -27,16 +27,17 @@ func planalyzeCmd() *cobra.Command {
 
 	cmd := &cobra.Command{
 		Use:     "planalyze",
-		Short:   "Analyze the query plan",
-		Example: "vt planalyze --vcshema-file file.vschema keys-log.json",
+		Short:   "Analyze the query plans using the keys output",
+		Long:    "Analyze the query plans. The report will report how many queries fall into one of the four categories: `passthrough`, `simple-routed`, `complex`, `unplannable`.",
+		Example: "vt planalyze --vcshema file.vschema keys-log.json",
 		Args:    cobra.ExactArgs(1),
 		RunE: func(_ *cobra.Command, args []string) error {
 			return planalyze.Run(cfg, args[0])
 		},
 	}
 
-	cmd.Flags().StringVar(&cfg.VSchemaFile, "vschema", "", "Disable auto-vschema by providing your own vschema file. This cannot be used with either -vtexplain-vschema or -sharded.")
-	cmd.Flags().StringVar(&cfg.VtExplainVschemaFile, "vtexplain-vschema", "", "Disable auto-vschema by providing your own vtexplain vschema file. This cannot be used with either -vschema or -sharded.")
+	cmd.Flags().StringVar(&cfg.VSchemaFile, "vschema", "", "Supply the vschema in a format that can contain multiple keyspaces. This cannot be used with -vtexplain-vschema.")
+	cmd.Flags().StringVar(&cfg.VtExplainVschemaFile, "vtexplain-vschema", "", "Supply the vschema in a format that contains a single keyspace")
 
 	return cmd
 }
diff --git a/go/data/csv_log_parse.go b/go/data/csv_log_parse.go
@@ -18,6 +18,7 @@ package data
 
 import (
 	"encoding/csv"
+	"fmt"
 	"io"
 	"os"
 	"strconv"
@@ -81,14 +82,18 @@ func (c *csvLogReaderState) Next() (Query, bool) {
 
 	l, _ := c.reader.FieldPos(0)
 
+	helpfulPanic := func(err error, val string) {
+		panic(fmt.Sprintf("%s at line: %d for value: %s", err.Error(), l, val))
+	}
+
 	recordToInt := func(idx *int) int {
 		if idx == nil {
 			return 0
 		}
 		val := record[*idx]
 		i, err := strconv.Atoi(val)
 		if err != nil {
-			panic(err)
+			helpfulPanic(err, val)
 		}
 		return i
 	}
@@ -100,7 +105,7 @@ func (c *csvLogReaderState) Next() (Query, bool) {
 		val := record[*idx]
 		f, err := strconv.ParseFloat(val, 64)
 		if err != nil {
-			panic(err)
+			helpfulPanic(err, val)
 		}
 		return f
 	}
@@ -112,7 +117,7 @@ func (c *csvLogReaderState) Next() (Query, bool) {
 		val := record[*idx]
 		t, err := time.Parse(time.DateTime, val)
 		if err != nil {
-			panic(err)
+			helpfulPanic(err, val)
 		}
 		return t.Unix()
 	}

diff --git a/go/planalyze/planalyze.go b/go/planalyze/planalyze.go
@@ -91,7 +91,7 @@ func run(out io.Writer, cfg Config, logFile string) error {
 	a := cfg.VSchemaFile != ""
 	b := cfg.VtExplainVschemaFile != ""
 	if a == b {
-		return errors.New("specify exactly one of the following flags: -vschema, -vtexplain-vschema, -sharded")
+		return errors.New("specify exactly one of the following flags: -vschema or -vtexplain-vschema")
 	}
 
 	_, vschema, err := data.GetKeyspaces(cfg.VSchemaFile, cfg.VtExplainVschemaFile, "main", false)
@@ -123,20 +123,35 @@ func run(out io.Writer, cfg Config, logFile string) error {
 		plan, err = planbuilder.TestBuilder(query.QueryStructure, vw, "")
 
 		res := getPlanRes(err, plan)
-		description := engine.PrimitiveToPlanDescription(plan.Instructions, nil)
-		b := new(bytes.Buffer)
-		enc := json.NewEncoder(b)
-		enc.SetEscapeHTML(false)
-		enc.SetIndent("", "  ")
-		err = enc.Encode(description)
-		if err != nil {
-			return err
+		switch {
+		case res == Unplannable:
+			errBytes, jsonErr := json.Marshal(err.Error())
+			if jsonErr != nil {
+				return jsonErr
+			}
+			planalyzer.Queries[res] = append(planalyzer.Queries[res], AnalyzedQuery{
+				QueryStructure: query.QueryStructure,
+				Complexity:     res,
+				PlanOutput:     errBytes,
+			})
+		case plan.Instructions != nil:
+			description := engine.PrimitiveToPlanDescription(plan.Instructions, nil)
+			b := new(bytes.Buffer)
+			enc := json.NewEncoder(b)
+			enc.SetEscapeHTML(false)
+			enc.SetIndent("", "  ")
+			err = enc.Encode(description)
+			if err != nil {
+				return err
+			}
+			planalyzer.Queries[res] = append(planalyzer.Queries[res], AnalyzedQuery{
+				QueryStructure: query.QueryStructure,
+				Complexity:     res,
+				PlanOutput:     json.RawMessage(b.String()),
+			})
+		default:
+			// if we don't have an instruction, this query is not interesting for planalyze
 		}
-		planalyzer.Queries[res] = append(planalyzer.Queries[res], AnalyzedQuery{
-			QueryStructure: query.QueryStructure,
-			Complexity:     res,
-			PlanOutput:     json.RawMessage(b.String()),
-		})
 	}
 
 	type jsonOutput struct {
@@ -171,11 +186,25 @@ func getPlanRes(err error, plan *engine.Plan) PlanComplexity {
 		return Unplannable
 	}
 
-	rb, ok := plan.Instructions.(*engine.Route)
-	switch {
-	case !ok:
+	var rp *engine.RoutingParameters
+
+	switch prim := plan.Instructions.(type) {
+	case *engine.Route:
+		rp = prim.RoutingParameters
+	case *engine.Update:
+		rp = prim.RoutingParameters
+	case *engine.Delete:
+		rp = prim.RoutingParameters
+	case *engine.Insert:
+		if prim.InsertCommon.Opcode == engine.InsertUnsharded {
+			return PassThrough
+		}
+		return SimpleRouted
+	default:
 		return Complex
-	case rb.Opcode.IsSingleShard():
+	}
+
+	if rp.Opcode.IsSingleShard() {
 		return PassThrough
 	}
 

diff --git a/go/testdata/keys-output/bigger_slow_query_log.json b/go/testdata/keys-output/bigger_slow_query_log.json
@@ -316,6 +316,14 @@
         "rowsSent": 15,
         "rowsExamined": 6500,
         "timestamp": 1690891224
+      },
+      {
+        "queryStructure": "COMMIT",
+        "usageCount": 1,
+        "lineNumbers": [
+          220
+        ],
+        "statementType": "COMMIT"
       }
     ],
     "failed": [

diff --git a/go/testdata/query-logs/bigger_slow_query_log.log b/go/testdata/query-logs/bigger_slow_query_log.log
@@ -216,4 +216,5 @@ SET timestamp=1690891225;
 SELECT m.sender_id, COUNT(DISTINCT m.receiver_id) as unique_receivers
 FROM messages m
 GROUP BY m.sender_id
-HAVING unique_receivers > 15;
+HAVING unique_receivers > 15;
+commit;