From 33a80cf97c8735e136603f39c7120b3a429256e0 Mon Sep 17 00:00:00 2001 From: Andrew Mason Date: Sun, 7 Mar 2021 16:24:07 -0500 Subject: [PATCH 1/4] Wrap errors across vtadmin to provide additional context on error conditions Signed-off-by: Andrew Mason --- go/vt/vtadmin/api.go | 25 +++++++++---------- go/vt/vtadmin/cluster/cluster.go | 10 ++++---- .../cluster/discovery/discovery_consul.go | 19 +++++++------- .../discovery/discovery_static_file.go | 3 ++- go/vt/vtadmin/vtctldclient/proxy.go | 5 ++-- go/vt/vtadmin/vtsql/vtsql.go | 4 +-- 6 files changed, 34 insertions(+), 32 deletions(-) diff --git a/go/vt/vtadmin/api.go b/go/vt/vtadmin/api.go index 7355c97d7c5..08bd5ccb4bb 100644 --- a/go/vt/vtadmin/api.go +++ b/go/vt/vtadmin/api.go @@ -254,7 +254,7 @@ func (api *API) GetGates(ctx context.Context, req *vtadminpb.GetGatesRequest) (* gs, err := c.Discovery.DiscoverVTGates(ctx, []string{}) if err != nil { - er.RecordError(err) + er.RecordError(fmt.Errorf("DiscoverVTGates(cluster = %s): %w", c.ID, err)) return } @@ -315,7 +315,7 @@ func (api *API) GetKeyspaces(ctx context.Context, req *vtadminpb.GetKeyspacesReq resp, err := c.Vtctld.GetKeyspaces(ctx, &vtctldatapb.GetKeyspacesRequest{}) if err != nil { - er.RecordError(err) + er.RecordError(fmt.Errorf("GetKeyspaces(cluster = %s): %w", c.ID, err)) return } @@ -337,7 +337,7 @@ func (api *API) GetKeyspaces(ctx context.Context, req *vtadminpb.GetKeyspacesReq }) if err != nil { - er.RecordError(err) + er.RecordError(fmt.Errorf("FindAllShardsInKeyspace(%s): %w", ks.Name, err)) return } @@ -566,7 +566,7 @@ func (api *API) GetTablet(ctx context.Context, req *vtadminpb.GetTabletRequest) ts, err := c.GetTablets(ctx) if err != nil { - er.RecordError(err) + er.RecordError(fmt.Errorf("GetTablets(cluster = %s): %w", c.ID, err)) return } @@ -622,7 +622,7 @@ func (api *API) GetTablets(ctx context.Context, req *vtadminpb.GetTabletsRequest ts, err := c.GetTablets(ctx) if err != nil { - er.RecordError(err) + er.RecordError(fmt.Errorf("GetTablets(cluster = %s): %w", c.ID, err)) return } @@ -796,15 +796,14 @@ func (api *API) VTExplain(ctx context.Context, req *vtadminpb.VTExplainRequest) c, ok := api.clusterMap[req.Cluster] if !ok { - return nil, errors.ErrUnsupportedCluster + return nil, fmt.Errorf("%w: %s", errors.ErrUnsupportedCluster, req.Cluster) } tablet, err := c.FindTablet(ctx, func(t *vtadminpb.Tablet) bool { return t.Tablet.Keyspace == req.Keyspace && topo.IsInServingGraph(t.Tablet.Type) && t.Tablet.Type != topodatapb.TabletType_MASTER && t.State == vtadminpb.Tablet_SERVING }) - if err != nil { - return nil, err + return nil, fmt.Errorf("cannot find serving, non-MASTER tablet in keyspace=%s: %w", req.Keyspace, err) } if err := c.Vtctld.Dial(ctx); err != nil { @@ -836,7 +835,7 @@ func (api *API) VTExplain(ctx context.Context, req *vtadminpb.VTExplainRequest) }) if err != nil { - er.RecordError(err) + er.RecordError(fmt.Errorf("GetSchema(%s): %w", topoproto.TabletAliasString(tablet.Tablet.Alias), err)) return } @@ -857,7 +856,7 @@ func (api *API) VTExplain(ctx context.Context, req *vtadminpb.VTExplainRequest) }) if err != nil { - er.RecordError(err) + er.RecordError(fmt.Errorf("GetSrvVSchema(%s): %w", tablet.Tablet.Alias.Cell, err)) return } @@ -885,7 +884,7 @@ func (api *API) VTExplain(ctx context.Context, req *vtadminpb.VTExplainRequest) }) if err != nil { - er.RecordError(err) + er.RecordError(fmt.Errorf("FindAllShardsInKeyspace(%s): %w", req.Keyspace, err)) return } @@ -912,12 +911,12 @@ func (api *API) VTExplain(ctx context.Context, req *vtadminpb.VTExplainRequest) opts := &vtexplain.Options{ReplicationMode: "ROW"} if err := vtexplain.Init(srvVSchema, schema, shardMap, opts); err != nil { - return nil, err + return nil, fmt.Errorf("error initilaizing vtexplain: %w", err) } plans, err := vtexplain.Run(req.Sql) if err != nil { - return nil, err + return nil, fmt.Errorf("error running vtexplain: %w", err) } response := vtexplain.ExplainsAsText(plans) diff --git a/go/vt/vtadmin/cluster/cluster.go b/go/vt/vtadmin/cluster/cluster.go index 5441fa6f540..eb051db115a 100644 --- a/go/vt/vtadmin/cluster/cluster.go +++ b/go/vt/vtadmin/cluster/cluster.go @@ -65,7 +65,7 @@ func New(cfg Config) (*Cluster, error) { disco, err := discovery.New(cfg.DiscoveryImpl, cluster.ToProto(), discoargs) if err != nil { - return nil, fmt.Errorf("error while creating discovery impl (%s): %w", cfg.DiscoveryImpl, err) + return nil, fmt.Errorf("error creating discovery impl (%s): %w", cfg.DiscoveryImpl, err) } cluster.Discovery = disco @@ -76,14 +76,14 @@ func New(cfg Config) (*Cluster, error) { vtsqlCfg, err := vtsql.Parse(protocluster, disco, vtsqlargs) if err != nil { - return nil, fmt.Errorf("error while creating vtsql connection config: %w", err) + return nil, fmt.Errorf("error creating vtsql connection config: %w", err) } vtctldargs := buildPFlagSlice(cfg.VtctldFlags) vtctldCfg, err := vtctldclient.Parse(protocluster, disco, vtctldargs) if err != nil { - return nil, fmt.Errorf("error while creating vtctldclient proxy config: %w", err) + return nil, fmt.Errorf("error creating vtctldclient proxy config: %w", err) } cluster.DB = vtsql.New(vtsqlCfg) @@ -179,7 +179,7 @@ func (c *Cluster) parseTablet(rows *sql.Rows) (*vtadminpb.Tablet, error) { topotablet.Alias, err = topoproto.ParseTabletAlias(aliasStr) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to parse tablet_alias %s: %w", aliasStr, err) } if topotablet.Alias.Cell != cell { @@ -190,7 +190,7 @@ func (c *Cluster) parseTablet(rows *sql.Rows) (*vtadminpb.Tablet, error) { if mtstStr != "" { timeTime, err := time.Parse(time.RFC3339, mtstStr) if err != nil { - return nil, err + return nil, fmt.Errorf("failed parsing master_term_start_time %s: %w", mtstStr, err) } topotablet.MasterTermStartTime = logutil.TimeToProto(timeTime) diff --git a/go/vt/vtadmin/cluster/discovery/discovery_consul.go b/go/vt/vtadmin/cluster/discovery/discovery_consul.go index 48ce42203cc..ddfdda27c8d 100644 --- a/go/vt/vtadmin/cluster/discovery/discovery_consul.go +++ b/go/vt/vtadmin/cluster/discovery/discovery_consul.go @@ -19,6 +19,7 @@ package discovery import ( "bytes" "context" + "fmt" "math/rand" "strings" "text/template" @@ -113,25 +114,25 @@ func NewConsul(cluster *vtadminpb.Cluster, flags *pflag.FlagSet, args []string) if *vtgateDatacenterTmplStr != "" { disco.vtgateDatacenter, err = generateConsulDatacenter("vtgate", cluster, *vtgateDatacenterTmplStr) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to generate vtgate consul datacenter from template: %w", err) } } disco.vtgateAddrTmpl, err = template.New("consul-vtgate-address-template-" + cluster.Id).Parse(*vtgateAddrTmplStr) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to parse vtgate host address template %s: %w", *vtgateAddrTmplStr, err) } if *vtctldDatacenterTmplStr != "" { disco.vtctldDatacenter, err = generateConsulDatacenter("vtctld", cluster, *vtctldDatacenterTmplStr) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to generate vtctld consul datacenter from template: %w", err) } } disco.vtctldAddrTmpl, err = template.New("consul-vtctld-address-template-" + cluster.Id).Parse(*vtctldAddrTmplStr) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to parse vtctld host address template %s: %w", *vtctldAddrTmplStr, err) } return disco, nil @@ -140,7 +141,7 @@ func NewConsul(cluster *vtadminpb.Cluster, flags *pflag.FlagSet, args []string) func generateConsulDatacenter(component string, cluster *vtadminpb.Cluster, tmplStr string) (string, error) { tmpl, err := template.New("consul-" + component + "-datacenter-" + cluster.Id).Parse(tmplStr) if err != nil { - return "", err + return "", fmt.Errorf("error parsing template %s: %w", tmplStr, err) } buf := bytes.NewBuffer(nil) @@ -151,10 +152,10 @@ func generateConsulDatacenter(component string, cluster *vtadminpb.Cluster, tmpl }) if err != nil { - return "", err + return "", fmt.Errorf("failed to execute template: %w", err) } - return buf.String(), err + return buf.String(), nil } // DiscoverVTGate is part of the Discovery interface. @@ -190,7 +191,7 @@ func (c *ConsulDiscovery) DiscoverVTGateAddr(ctx context.Context, tags []string) buf := bytes.NewBuffer(nil) if err := c.vtgateAddrTmpl.Execute(buf, vtgate); err != nil { - return "", err + return "", fmt.Errorf("failed to execute vtgate address template for %v: %w", vtgate, err) } return buf.String(), nil @@ -290,7 +291,7 @@ func (c *ConsulDiscovery) DiscoverVtctldAddr(ctx context.Context, tags []string) buf := bytes.NewBuffer(nil) if err := c.vtctldAddrTmpl.Execute(buf, vtctld); err != nil { - return "", err + return "", fmt.Errorf("failed to execute vtctld address template for %v: %w", vtctld, err) } return buf.String(), nil diff --git a/go/vt/vtadmin/cluster/discovery/discovery_static_file.go b/go/vt/vtadmin/cluster/discovery/discovery_static_file.go index 653b526e3cd..62786858403 100644 --- a/go/vt/vtadmin/cluster/discovery/discovery_static_file.go +++ b/go/vt/vtadmin/cluster/discovery/discovery_static_file.go @@ -20,6 +20,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io/ioutil" "math/rand" @@ -106,7 +107,7 @@ func NewStaticFile(cluster *vtadminpb.Cluster, flags *pflag.FlagSet, args []stri func (d *StaticFileDiscovery) parseConfig(bytes []byte) error { if err := json.Unmarshal(bytes, &d.config); err != nil { - return err + return fmt.Errorf("failed to unmarshal staticfile config from json: %w", err) } d.gates.byName = make(map[string]*vtadminpb.VTGate, len(d.config.VTGates)) diff --git a/go/vt/vtadmin/vtctldclient/proxy.go b/go/vt/vtadmin/vtctldclient/proxy.go index c90e698def5..bbf62d64d0f 100644 --- a/go/vt/vtadmin/vtctldclient/proxy.go +++ b/go/vt/vtadmin/vtctldclient/proxy.go @@ -18,6 +18,7 @@ package vtctldclient import ( "context" + "fmt" "google.golang.org/grpc" @@ -92,13 +93,13 @@ func (vtctld *ClientProxy) Dial(ctx context.Context) error { // close before reopen. this is safe to call on an already-closed client. if err := vtctld.Close(); err != nil { - return err + return fmt.Errorf("error closing possibly-stale connection before re-dialing: %w", err) } } addr, err := vtctld.discovery.DiscoverVtctldAddr(ctx, nil) if err != nil { - return err + return fmt.Errorf("error discovering vtctld to dial: %w", err) } opts := []grpc.DialOption{} diff --git a/go/vt/vtadmin/vtsql/vtsql.go b/go/vt/vtadmin/vtsql/vtsql.go index 82782215190..c86f86074aa 100644 --- a/go/vt/vtadmin/vtsql/vtsql.go +++ b/go/vt/vtadmin/vtsql/vtsql.go @@ -139,7 +139,7 @@ func (vtgate *VTGateProxy) Dial(ctx context.Context, target string, opts ...grpc if vtgate.host == "" { gate, err := vtgate.discovery.DiscoverVTGateAddr(ctx, vtgate.discoveryTags) if err != nil { - return err + return fmt.Errorf("error discovering vtgate to dial: %w", err) } vtgate.host = gate @@ -163,7 +163,7 @@ func (vtgate *VTGateProxy) Dial(ctx context.Context, target string, opts ...grpc db, err := vtgate.DialFunc(conf) if err != nil { - return err + return fmt.Errorf("error dialing vtgate %s: %w", vtgate.host, err) } vtgate.conn = db From ec554a650ee6821cdee986ac7929564757908b13 Mon Sep 17 00:00:00 2001 From: Andrew Mason Date: Sun, 7 Mar 2021 16:25:08 -0500 Subject: [PATCH 2/4] Switch vtexplain to use the cluster-level GetSchema for more code-sharing Signed-off-by: Andrew Mason --- go/vt/vtadmin/api.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/go/vt/vtadmin/api.go b/go/vt/vtadmin/api.go index 08bd5ccb4bb..54cd6798c43 100644 --- a/go/vt/vtadmin/api.go +++ b/go/vt/vtadmin/api.go @@ -830,17 +830,14 @@ func (api *API) VTExplain(ctx context.Context, req *vtadminpb.VTExplainRequest) go func(c *cluster.Cluster) { defer wg.Done() - res, err := c.Vtctld.GetSchema(ctx, &vtctldatapb.GetSchemaRequest{ - TabletAlias: tablet.Tablet.Alias, - }) - + res, err := c.GetSchema(ctx, &vtctldatapb.GetSchemaRequest{}, tablet) if err != nil { er.RecordError(fmt.Errorf("GetSchema(%s): %w", topoproto.TabletAliasString(tablet.Tablet.Alias), err)) return } - schemas := make([]string, len(res.Schema.TableDefinitions)) - for i, td := range res.Schema.TableDefinitions { + schemas := make([]string, len(res.TableDefinitions)) + for i, td := range res.TableDefinitions { schemas[i] = td.Schema } From 2f1956e997f9d6bcc2479d760873008ff0eb4bf5 Mon Sep 17 00:00:00 2001 From: Andrew Mason Date: Sun, 7 Mar 2021 16:26:11 -0500 Subject: [PATCH 3/4] Rename staticFile=>staticfile to match the go/vitess _style_ Signed-off-by: Andrew Mason --- go/vt/vtadmin/cluster/discovery/discovery.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/vtadmin/cluster/discovery/discovery.go b/go/vt/vtadmin/cluster/discovery/discovery.go index 8a2f9e50217..616c29a9f37 100644 --- a/go/vt/vtadmin/cluster/discovery/discovery.go +++ b/go/vt/vtadmin/cluster/discovery/discovery.go @@ -109,5 +109,5 @@ func New(impl string, cluster *vtadminpb.Cluster, args []string) (Discovery, err func init() { // nolint:gochecknoinits Register("consul", NewConsul) - Register("staticFile", NewStaticFile) + Register("staticfile", NewStaticFile) } From ba2ffa75733e0ee992c62459c6571b7128f1b366 Mon Sep 17 00:00:00 2001 From: Andrew Mason Date: Mon, 8 Mar 2021 16:34:59 -0500 Subject: [PATCH 4/4] Replace "master" with "primary" Signed-off-by: Andrew Mason --- go/vt/vtadmin/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/vtadmin/api.go b/go/vt/vtadmin/api.go index 54cd6798c43..32ce277eaaa 100644 --- a/go/vt/vtadmin/api.go +++ b/go/vt/vtadmin/api.go @@ -803,7 +803,7 @@ func (api *API) VTExplain(ctx context.Context, req *vtadminpb.VTExplainRequest) return t.Tablet.Keyspace == req.Keyspace && topo.IsInServingGraph(t.Tablet.Type) && t.Tablet.Type != topodatapb.TabletType_MASTER && t.State == vtadminpb.Tablet_SERVING }) if err != nil { - return nil, fmt.Errorf("cannot find serving, non-MASTER tablet in keyspace=%s: %w", req.Keyspace, err) + return nil, fmt.Errorf("cannot find serving, non-primary tablet in keyspace=%s: %w", req.Keyspace, err) } if err := c.Vtctld.Dial(ctx); err != nil {