Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vtadmin] Add GetSchema endpoint #7596

Merged
merged 7 commits into from
Mar 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion go/vt/proto/topodata/topodata.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

450 changes: 389 additions & 61 deletions go/vt/proto/vtadmin/vtadmin.pb.go

Large diffs are not rendered by default.

52 changes: 39 additions & 13 deletions go/vt/vtadmin/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"vitess.io/vitess/go/trace"
"vitess.io/vitess/go/vt/concurrency"
"vitess.io/vitess/go/vt/topo"
"vitess.io/vitess/go/vt/topo/topoproto"
"vitess.io/vitess/go/vt/vtadmin/cluster"
"vitess.io/vitess/go/vt/vtadmin/errors"
"vitess.io/vitess/go/vt/vtadmin/grpcserver"
Expand Down Expand Up @@ -87,6 +88,7 @@ func NewAPI(clusters []*cluster.Cluster, opts grpcserver.Options, httpOpts vtadm
router.HandleFunc("/clusters", httpAPI.Adapt(vtadminhttp.GetClusters)).Name("API.GetClusters")
router.HandleFunc("/gates", httpAPI.Adapt(vtadminhttp.GetGates)).Name("API.GetGates")
router.HandleFunc("/keyspaces", httpAPI.Adapt(vtadminhttp.GetKeyspaces)).Name("API.GetKeyspaces")
router.HandleFunc("/schema/{cluster_id}/{keyspace}/{table}", httpAPI.Adapt(vtadminhttp.GetSchema)).Name("API.GetSchema")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about making this /schema?cluster_id=&keyspace=&table=? I honestly think the way you have it now is prettier, but:

  • It would also work for disambiguation (where cluster/keyspace are optinoal)
  • We already do this for the /tablet endpoint (which has an optional cluster parameter too)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize you started the disambiguation branch anyway so I expect you already have some plan 😎

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, given that you said this route scheme looks prettier, that we should keep it as-is because:

  1. we're still able to reuse the logic of cluster.GetSchema between the two endpoints, so we're not duplicating too much
  2. having two distinct rpcs and http routes lets us move some of the conditional logic out of the actual function and into the route definition (i.e. GetSchema doesn't have to that len(clusterIds) is exactly 1, because it's not possible, but if we combine it with the FindSchema endpoint then we'll end up introducing a bunch of "if this then do thingAForGetSchema else do thingBForFindSchema" branching)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah makes sense!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /schema endpoint does seem a bit non-std instead of url params or nested resource paths like /schema/cluster/{cluster_id}/keyspace/{keyspace}/table/{table}. But it is more succinct and we can revisit this once we have all our endpoints in place.

router.HandleFunc("/schemas", httpAPI.Adapt(vtadminhttp.GetSchemas)).Name("API.GetSchemas")
router.HandleFunc("/tablets", httpAPI.Adapt(vtadminhttp.GetTablets)).Name("API.GetTablets")
router.HandleFunc("/tablet/{tablet}", httpAPI.Adapt(vtadminhttp.GetTablet)).Name("API.GetTablet")
Expand Down Expand Up @@ -280,6 +282,40 @@ func (api *API) GetKeyspaces(ctx context.Context, req *vtadminpb.GetKeyspacesReq
}, nil
}

// GetSchema is part of the vtadminpb.VTAdminServer interface.
func (api *API) GetSchema(ctx context.Context, req *vtadminpb.GetSchemaRequest) (*vtadminpb.Schema, error) {
span, ctx := trace.NewSpan(ctx, "API.GetSchema")
defer span.Finish()

span.Annotate("cluster_id", req.ClusterId)
span.Annotate("keyspace", req.Keyspace)
span.Annotate("table", req.Table)

clusters, _ := api.getClustersForRequest([]string{req.ClusterId})
if len(clusters) == 0 {
return nil, fmt.Errorf("%w: no cluster with id %s", errors.ErrUnsupportedCluster, req.ClusterId)
}

cluster := clusters[0]

tablet, err := cluster.FindTablet(ctx, func(t *vtadminpb.Tablet) bool {
return t.Tablet.Keyspace == req.Keyspace && t.State == vtadminpb.Tablet_SERVING
})
if err != nil {
return nil, fmt.Errorf("%w: no serving tablet found for keyspace %s", err, req.Keyspace)
}

span.Annotate("tablet_alias", topoproto.TabletAliasString(tablet.Tablet.Alias))

if err := cluster.Vtctld.Dial(ctx); err != nil {
return nil, err
}

return cluster.GetSchema(ctx, &vtctldatapb.GetSchemaRequest{
Tables: []string{req.Table},
}, tablet)
}

// GetSchemas is part of the vtadminpb.VTAdminServer interface.
func (api *API) GetSchemas(ctx context.Context, req *vtadminpb.GetSchemasRequest) (*vtadminpb.GetSchemasResponse, error) {
span, ctx := trace.NewSpan(ctx, "API.GetSchemas")
Expand Down Expand Up @@ -403,29 +439,19 @@ func (api *API) getSchemasForKeyspace(ctx context.Context, c *cluster.Cluster, k
return nil, err
}

s, err := c.Vtctld.GetSchema(ctx, &vtctldatapb.GetSchemaRequest{
TabletAlias: kt.Tablet.Alias,
})

s, err := c.GetSchema(ctx, &vtctldatapb.GetSchemaRequest{}, kt)
if err != nil {
return nil, err
}

// Ignore any schemas without table definitions; otherwise we return
// a vtadminpb.Schema object with only Cluster and Keyspace defined,
// which is not particularly useful.
if s == nil || s.Schema == nil || len(s.Schema.TableDefinitions) == 0 {
if s == nil || len(s.TableDefinitions) == 0 {
return nil, nil
}

return &vtadminpb.Schema{
Cluster: &vtadminpb.Cluster{
Id: c.ID,
Name: c.Name,
},
Keyspace: ks.Name,
TableDefinitions: s.Schema.TableDefinitions,
}, nil
return s, nil
}

// GetTablet is part of the vtadminpb.VTAdminServer interface.
Expand Down
221 changes: 221 additions & 0 deletions go/vt/vtadmin/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,227 @@ func TestGetKeyspaces(t *testing.T) {
}
}

func TestGetSchema(t *testing.T) {
t.Parallel()

tests := []struct {
name string
clusterID int
ts *topo.Server
tmc tmclient.TabletManagerClient
tablets []*vtadminpb.Tablet
req *vtadminpb.GetSchemaRequest
expected *vtadminpb.Schema
shouldErr bool
}{
{
name: "success",
clusterID: 1,
ts: memorytopo.NewServer("zone1"),
tmc: &testutil.TabletManagerClient{
GetSchemaResults: map[string]struct {
Schema *tabletmanagerdatapb.SchemaDefinition
Error error
}{
"zone1-0000000100": {
Schema: &tabletmanagerdatapb.SchemaDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "testtable",
},
},
},
},
},
},
tablets: []*vtadminpb.Tablet{
{
Cluster: &vtadminpb.Cluster{
Id: "c1",
Name: "cluster1",
},
State: vtadminpb.Tablet_SERVING,
Tablet: &topodatapb.Tablet{
Alias: &topodatapb.TabletAlias{
Cell: "zone1",
Uid: 100,
},
Keyspace: "testkeyspace",
},
},
},
req: &vtadminpb.GetSchemaRequest{
ClusterId: "c1",
Keyspace: "testkeyspace",
Table: "testtable",
},
expected: &vtadminpb.Schema{
Cluster: &vtadminpb.Cluster{
Id: "c1",
Name: "cluster1",
},
Keyspace: "testkeyspace",
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "testtable",
},
},
},
shouldErr: false,
},
{
name: "cluster not found",
clusterID: 1, // results in clusterId == "c1"
ts: memorytopo.NewServer("zone1"),
tablets: nil,
req: &vtadminpb.GetSchemaRequest{
ClusterId: "c2",
Keyspace: "testkeyspace",
Table: "testtable",
},
expected: nil,
shouldErr: true,
},
{
name: "tablet not found for keyspace",
clusterID: 1,
ts: memorytopo.NewServer("zone1"),
tablets: []*vtadminpb.Tablet{
{
Cluster: &vtadminpb.Cluster{
Id: "c1",
Name: "cluster1",
},
State: vtadminpb.Tablet_SERVING,
Tablet: &topodatapb.Tablet{
Alias: &topodatapb.TabletAlias{
Cell: "zone1",
Uid: 100,
},
Keyspace: "otherkeyspace",
},
},
},
req: &vtadminpb.GetSchemaRequest{
ClusterId: "c1",
Keyspace: "testkeyspace",
Table: "testtable",
},
expected: nil,
shouldErr: true,
},
{
name: "no serving tablet found for keyspace",
clusterID: 1,
ts: memorytopo.NewServer("zone1"),
tablets: []*vtadminpb.Tablet{
{
Cluster: &vtadminpb.Cluster{
Id: "c1",
Name: "cluster1",
},
State: vtadminpb.Tablet_NOT_SERVING,
Tablet: &topodatapb.Tablet{
Alias: &topodatapb.TabletAlias{
Cell: "zone1",
Uid: 100,
},
Keyspace: "testkeyspace",
},
},
},
req: &vtadminpb.GetSchemaRequest{
ClusterId: "c1",
Keyspace: "testkeyspace",
Table: "testtable",
},
expected: nil,
shouldErr: true,
},
{
name: "error in GetSchema call",
clusterID: 1,
ts: memorytopo.NewServer("zone1"),
tmc: &testutil.TabletManagerClient{
GetSchemaResults: map[string]struct {
Schema *tabletmanagerdatapb.SchemaDefinition
Error error
}{
"zone1-0000000100": {
Schema: &tabletmanagerdatapb.SchemaDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "testtable",
},
{
Name: "table2",
},
{
Name: "table3",
},
},
},
Error: assert.AnError,
},
},
},
tablets: []*vtadminpb.Tablet{
{
Cluster: &vtadminpb.Cluster{
Id: "c1",
Name: "cluster1",
},
State: vtadminpb.Tablet_SERVING,
Tablet: &topodatapb.Tablet{
Alias: &topodatapb.TabletAlias{
Cell: "zone1",
Uid: 100,
},
Keyspace: "testkeyspace",
},
},
},
req: &vtadminpb.GetSchemaRequest{
ClusterId: "c1",
Keyspace: "testkeyspace",
Table: "testtable",
},
expected: nil,
shouldErr: true,
},
}

for _, tt := range tests {
tt := tt

t.Run(tt.name, func(t *testing.T) {
t.Parallel()

ctx := context.Background()
vtctld := testutil.NewVtctldServerWithTabletManagerClient(t, tt.ts, tt.tmc, func(ts *topo.Server) vtctlservicepb.VtctldServer {
return grpcvtctldserver.NewVtctldServer(ts)
})

testutil.AddTablets(ctx, t, tt.ts, nil, vtadmintestutil.TopodataTabletsFromVTAdminTablets(tt.tablets)...)

testutil.WithTestServer(t, vtctld, func(t *testing.T, client vtctldclient.VtctldClient) {
c := vtadmintestutil.BuildCluster(tt.clusterID, client, tt.tablets, nil)
api := NewAPI([]*cluster.Cluster{c}, grpcserver.Options{}, http.Options{})

resp, err := api.GetSchema(ctx, tt.req)
if tt.shouldErr {
assert.Error(t, err)

return
}

assert.NoError(t, err)
assert.Equal(t, tt.expected, resp)
})
})
}
}

func TestGetSchemas(t *testing.T) {
t.Parallel()

Expand Down
35 changes: 35 additions & 0 deletions go/vt/vtadmin/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (

topodatapb "vitess.io/vitess/go/vt/proto/topodata"
vtadminpb "vitess.io/vitess/go/vt/proto/vtadmin"
vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
)

// Cluster is the self-contained unit of services required for vtadmin to talk
Expand Down Expand Up @@ -211,6 +212,40 @@ func (c *Cluster) GetTablets(ctx context.Context) ([]*vtadminpb.Tablet, error) {
return c.parseTablets(rows)
}

// GetSchema returns the schema for a GetSchemaRequest on the given tablet. The
// caller is responsible for making at least one call to c.Vtctld.Dial prior to
// calling this function.
//
// Note that the request's TabletAlias field will be ignored, using the provided
// tablet's Alias instead. This override is done on a copy of the request, so it
// is transparent to the caller.
//
// This function takes both the request argument and tablet argument to
// (a) set the Keyspace field on the resulting Schema object, which comes from
// the provided tablet; and, (b) allow a caller, like vtadmin.API to collect a
// bunch of tablets once and make a series of GetSchema calls without Cluster
// refetching the tablet list each time.
func (c *Cluster) GetSchema(ctx context.Context, req *vtctldatapb.GetSchemaRequest, tablet *vtadminpb.Tablet) (*vtadminpb.Schema, error) {
// Copy the request to not mutate the caller's request object.
r := *req
r.TabletAlias = tablet.Tablet.Alias

schema, err := c.Vtctld.GetSchema(ctx, &r)
if err != nil {
return nil, err
}

if schema == nil || schema.Schema == nil {
return nil, nil
}

return &vtadminpb.Schema{
Cluster: c.ToProto(),
Keyspace: tablet.Tablet.Keyspace,
TableDefinitions: schema.Schema.TableDefinitions,
}, nil
}

// FindTablet returns the first tablet in a given cluster that satisfies the filter function.
func (c *Cluster) FindTablet(ctx context.Context, filter func(*vtadminpb.Tablet) bool) (*vtadminpb.Tablet, error) {
tablets, err := c.FindTablets(ctx, filter, 1)
Expand Down
Loading