From e637cefa03ede35eb2aeeccd3eae23b183899b45 Mon Sep 17 00:00:00 2001 From: Andrew Mason Date: Wed, 7 Apr 2021 20:38:50 -0400 Subject: [PATCH 1/2] Update `GetSchema` filtering to exclude shards where `IsMasterServing` but no `MasterAlias` Signed-off-by: Andrew Mason --- go/vt/vtadmin/api_test.go | 40 +++++++++++++++++++++++++++ go/vt/vtadmin/cluster/cluster.go | 2 +- go/vt/vtadmin/cluster/cluster_test.go | 40 +++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/go/vt/vtadmin/api_test.go b/go/vt/vtadmin/api_test.go index dd895fb3962..bcd391ddfb7 100644 --- a/go/vt/vtadmin/api_test.go +++ b/go/vt/vtadmin/api_test.go @@ -557,6 +557,10 @@ func TestFindSchema(t *testing.T) { Name: "-80", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "c1zone1", + Uid: 100, + }, }, }, "80-": { @@ -564,6 +568,10 @@ func TestFindSchema(t *testing.T) { Name: "80-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "c1zone1", + Uid: 200, + }, }, }, }, @@ -577,6 +585,10 @@ func TestFindSchema(t *testing.T) { Name: "-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "c1zone1", + Uid: 300, + }, }, }, }, @@ -668,6 +680,10 @@ func TestFindSchema(t *testing.T) { Name: "-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "c2z1", + Uid: 100, + }, }, }, }, @@ -1403,6 +1419,10 @@ func TestGetSchema(t *testing.T) { Name: "-80", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "c1zone1", + Uid: 100, + }, }, }, "80-": { @@ -1410,6 +1430,10 @@ func TestGetSchema(t *testing.T) { Name: "80-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "c1zone1", + Uid: 200, + }, }, }, }, @@ -2073,6 +2097,10 @@ func TestGetSchemas(t *testing.T) { Name: "-80", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "c1zone1", + Uid: 100, + }, }, }, "80-": { @@ -2080,6 +2108,10 @@ func TestGetSchemas(t *testing.T) { Name: "80-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "c1zone1", + Uid: 200, + }, }, }, }, @@ -2093,6 +2125,10 @@ func TestGetSchemas(t *testing.T) { Name: "-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "c1zone2", + Uid: 100, + }, }, }, }, @@ -2184,6 +2220,10 @@ func TestGetSchemas(t *testing.T) { Name: "-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "c2z1", + Uid: 100, + }, }, }, }, diff --git a/go/vt/vtadmin/cluster/cluster.go b/go/vt/vtadmin/cluster/cluster.go index 26e8bba8335..34f366b486e 100644 --- a/go/vt/vtadmin/cluster/cluster.go +++ b/go/vt/vtadmin/cluster/cluster.go @@ -633,7 +633,7 @@ func (c *Cluster) getTabletsToQueryForSchemas(ctx context.Context, keyspace stri tabletsToQuery := make([]*vtadminpb.Tablet, 0, len(shards)) for _, shard := range shards { - if !shard.Shard.IsMasterServing { + if !(shard.Shard.IsMasterServing && shard.Shard.MasterAlias != nil) { log.Infof("%s/%s is not serving; ignoring ...", keyspace, shard.Name) continue } diff --git a/go/vt/vtadmin/cluster/cluster_test.go b/go/vt/vtadmin/cluster/cluster_test.go index 3246876c483..703da92c4e3 100644 --- a/go/vt/vtadmin/cluster/cluster_test.go +++ b/go/vt/vtadmin/cluster/cluster_test.go @@ -632,12 +632,20 @@ func TestGetSchema(t *testing.T) { Name: "-80", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 100, + }, }, }, "80-": { Name: "80-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 200, + }, }, }, "-": { @@ -769,12 +777,20 @@ func TestGetSchema(t *testing.T) { Name: "-80", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 100, + }, }, }, "80-": { Name: "80-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 200, + }, }, }, "-": { @@ -876,12 +892,20 @@ func TestGetSchema(t *testing.T) { Name: "-80", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 100, + }, }, }, "80-": { Name: "80-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 200, + }, }, }, "-": { @@ -1016,12 +1040,20 @@ func TestGetSchema(t *testing.T) { Name: "-80", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 100, + }, }, }, "80-": { Name: "80-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 200, + }, }, }, "-": { @@ -1197,12 +1229,20 @@ func TestGetSchema(t *testing.T) { Name: "-80", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 100, + }, }, }, "80-": { Name: "80-", Shard: &topodatapb.Shard{ IsMasterServing: true, + MasterAlias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 200, + }, }, }, }, From 658f78a3dbb082bae7f15ac6a5d1bdcf646363e9 Mon Sep 17 00:00:00 2001 From: Andrew Mason Date: Thu, 8 Apr 2021 11:25:18 -0400 Subject: [PATCH 2/2] Add comment Signed-off-by: Andrew Mason --- go/vt/vtadmin/cluster/cluster.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go/vt/vtadmin/cluster/cluster.go b/go/vt/vtadmin/cluster/cluster.go index 34f366b486e..25edbd883d5 100644 --- a/go/vt/vtadmin/cluster/cluster.go +++ b/go/vt/vtadmin/cluster/cluster.go @@ -633,6 +633,11 @@ func (c *Cluster) getTabletsToQueryForSchemas(ctx context.Context, keyspace stri tabletsToQuery := make([]*vtadminpb.Tablet, 0, len(shards)) for _, shard := range shards { + // In certain setups, empty but "serving" shards may required to + // provide a contiguous keyspace so that certain keyspace-level + // operations will work. In our case, we care about whether the + // shard is truly serving, which we define as also having a known + // primary (via MasterAlias) in addition to the IsMasterServing bit. if !(shard.Shard.IsMasterServing && shard.Shard.MasterAlias != nil) { log.Infof("%s/%s is not serving; ignoring ...", keyspace, shard.Name) continue