[vtadmin-api] Reintroduce include_non_serving_shards opt to GetSchema

go/vt/vtadmin/cluster/cluster.go

@@ -466,18 +466,27 @@ type GetSchemaOptions struct {
 // each shard. If this option is false, we make exactly one GetSchema request to
 // a single, randomly-chosen, tablet in the keyspace.
 //
-// (3) We will only make GetSchema RPCs to tablets that are in SERVING state; we
-// don't want to use a tablet that might be in a bad state as the source of
-// truth for a schema. Therefore if we can't find a SERVING tablet for the
-// keyspace (in non-aggregation mode) or for a shard in that keyspace (in
-// aggregation mode), then we will return an error back to the caller.
+// (2.1) If, in size aggregation mode, opts.SizeOpts.IncludeNonServingShards is
+// false (the default), then we will filter out any shards for which
+// IsMasterServing is false in the topo, and make GetSchema RPCs to one tablet
+// in every _serving_ shard. Otherwise we will make a GetSchema RPC to one
+// tablet in _every_ shard.
+//
+// (3) Irrespective of whether we're including nonserving shards, or whether
+// we're doing size aggregation at all, we will only make GetSchema RPCs to
+// tablets that are in SERVING state; we don't want to use a tablet that might
+// be in a bad state as the source of truth for a schema. Therefore if we can't
+// find a SERVING tablet for the keyspace (in non-aggregation mode) or for a
+// shard in that keyspace (in aggregation mode), then we will return an error
+// back to the caller.
 func (c *Cluster) GetSchema(ctx context.Context, keyspace string, opts GetSchemaOptions) (*vtadminpb.Schema, error) {
 	span, ctx := trace.NewSpan(ctx, "Cluster.GetSchema")
 	defer span.Finish()
 
 	if opts.TableSizeOptions == nil {
 		opts.TableSizeOptions = &vtadminpb.GetSchemaTableSizeOptions{
-			AggregateSizes: false,
+			AggregateSizes:          false,
+			IncludeNonServingShards: false,
 		}
 	}
 
@@ -638,9 +647,11 @@ func (c *Cluster) getTabletsToQueryForSchemas(ctx context.Context, keyspace stri
 			// 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
+			if !shard.Shard.IsMasterServing || shard.Shard.MasterAlias == nil {
+				if !opts.TableSizeOptions.IncludeNonServingShards {
+					log.Infof("%s/%s is not serving; ignoring because IncludeNonServingShards = false", keyspace, shard.Name)
+					continue
+				}
 			}
 
 			shardTablets := vtadminproto.FilterTablets(func(tablet *vtadminpb.Tablet) bool {

go/vt/vtadmin/cluster/cluster_test.go

@@ -734,6 +734,174 @@ func TestGetSchema(t *testing.T) {
 				},
 				shouldErr: false,
 			},
+			{
+				name: "include nonserving shards",
+				cfg: testutil.TestClusterConfig{
+					Cluster: &vtadminpb.Cluster{
+						Id:   "c0",
+						Name: "cluster0",
+					},
+					Tablets: []*vtadminpb.Tablet{
+						{
+							Tablet: &topodatapb.Tablet{
+								Alias: &topodatapb.TabletAlias{
+									Cell: "zone1",
+									Uid:  100,
+								},
+								Keyspace: "testkeyspace",
+								Shard:    "-80",
+							},
+							State: vtadminpb.Tablet_SERVING,
+						},
+						{
+							Tablet: &topodatapb.Tablet{
+								Alias: &topodatapb.TabletAlias{
+									Cell: "zone1",
+									Uid:  200,
+								},
+								Keyspace: "testkeyspace",
+								Shard:    "80-",
+							},
+							State: vtadminpb.Tablet_SERVING,
+						},
+						{
+							Tablet: &topodatapb.Tablet{
+								Alias: &topodatapb.TabletAlias{
+									Cell: "zone1",
+									Uid:  300,
+								},
+								Keyspace: "testkeyspace",
+								Shard:    "-",
+							},
+							State: vtadminpb.Tablet_SERVING,
+						},
+					},
+					VtctldClient: &testutil.VtctldClient{
+						FindAllShardsInKeyspaceResults: map[string]struct {
+							Response *vtctldatapb.FindAllShardsInKeyspaceResponse
+							Error    error
+						}{
+							"testkeyspace": {
+								Response: &vtctldatapb.FindAllShardsInKeyspaceResponse{
+									Shards: map[string]*vtctldatapb.Shard{
+										"-80": {
+											Name: "-80",
+											Shard: &topodatapb.Shard{
+												IsMasterServing: true,
+											},
+										},
+										"80-": {
+											Name: "80-",
+											Shard: &topodatapb.Shard{
+												IsMasterServing: true,
+											},
+										},
+										"-": {
+											Name: "-",
+											Shard: &topodatapb.Shard{
+												IsMasterServing: false,
+											},
+										},
+									},
+								},
+							},
+						},
+						GetSchemaResults: map[string]struct {
+							Response *vtctldatapb.GetSchemaResponse
+							Error    error
+						}{
+							"zone1-0000000100": {
+								Response: &vtctldatapb.GetSchemaResponse{
+									Schema: &tabletmanagerdatapb.SchemaDefinition{
+										DatabaseSchema: "CREATE DATABASE vt_testkeyspcae",
+										TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
+											{
+												Name:       "foo",
+												Schema:     "CREATE TABLE foo (\n\tid INT(11) NOT NULL\n) ENGINE=InnoDB",
+												DataLength: 100,
+												RowCount:   5,
+											},
+										},
+									},
+								},
+							},
+							"zone1-0000000200": {
+								Response: &vtctldatapb.GetSchemaResponse{
+									Schema: &tabletmanagerdatapb.SchemaDefinition{
+										DatabaseSchema: "CREATE DATABASE vt_testkeyspcae",
+										TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
+											{
+												Name:       "foo",
+												Schema:     "CREATE TABLE foo (\n\tid INT(11) NOT NULL\n) ENGINE=InnoDB",
+												DataLength: 200,
+												RowCount:   420,
+											},
+										},
+									},
+								},
+							},
+							"zone1-0000000300": {
+								Response: &vtctldatapb.GetSchemaResponse{
+									Schema: &tabletmanagerdatapb.SchemaDefinition{
+										DatabaseSchema: "CREATE DATABASE vt_testkeyspcae",
+										TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
+											{
+												Name:       "foo",
+												Schema:     "CREATE TABLE foo (\n\tid INT(11) NOT NULL\n) ENGINE=InnoDB",
+												DataLength: 1000,
+												RowCount:   200,
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+				keyspace: "testkeyspace",
+				opts: cluster.GetSchemaOptions{
+					TableSizeOptions: &vtadminpb.GetSchemaTableSizeOptions{
+						AggregateSizes:          true,
+						IncludeNonServingShards: true,
+					},
+				},
+				expected: &vtadminpb.Schema{
+					Cluster: &vtadminpb.Cluster{
+						Id:   "c0",
+						Name: "cluster0",
+					},
+					Keyspace: "testkeyspace",
+					TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
+						{
+							Name:       "foo",
+							Schema:     "CREATE TABLE foo (\n\tid INT(11) NOT NULL\n) ENGINE=InnoDB",
+							DataLength: 0,
+							RowCount:   0,
+						},
+					},
+					TableSizes: map[string]*vtadminpb.Schema_TableSize{
+						"foo": {
+							DataLength: 100 + 200 + 1000,
+							RowCount:   5 + 420 + 200,
+							ByShard: map[string]*vtadminpb.Schema_ShardTableSize{
+								"-80": {
+									DataLength: 100,
+									RowCount:   5,
+								},
+								"80-": {
+									DataLength: 200,
+									RowCount:   420,
+								},
+								"-": {
+									DataLength: 1000,
+									RowCount:   200,
+								},
+							},
+						},
+					},
+				},
+				shouldErr: false,
+			},
 			{
 				name: "no serving tablets found for shard",
 				cfg: testutil.TestClusterConfig{

go/vt/vtadmin/http/schemas.go

@@ -84,7 +84,13 @@ func getTableSizeOpts(r Request) (*vtadminpb.GetSchemaTableSizeOptions, error) {
 		return nil, err
 	}
 
+	includeNonServingShards, err := r.ParseQueryParamAsBool("include_non_serving_shards", false)
+	if err != nil {
+		return nil, err
+	}
+
 	return &vtadminpb.GetSchemaTableSizeOptions{
-		AggregateSizes: aggregateSizes,
+		AggregateSizes:          aggregateSizes,
+		IncludeNonServingShards: includeNonServingShards,
 	}, nil
 }

go/vt/vtadmin/vtadminproto/trace.go

@@ -36,4 +36,5 @@ func AnnotateSpanWithGetSchemaTableSizeOptions(opts *vtadminpb.GetSchemaTableSiz
 	}
 
 	span.Annotate("aggregate_table_sizes", opts.AggregateSizes)
+	span.Annotate("include_non_serving_shards", opts.IncludeNonServingShards)
 }

proto/vtadmin.proto

@@ -212,6 +212,7 @@ message GetSchemasResponse {
 
 message GetSchemaTableSizeOptions {
     bool aggregate_sizes = 1;
+    bool include_non_serving_shards = 2;
 }
 
 message GetTabletRequest {

web/vtadmin/src/proto/vtadmin.d.ts

@@ -2363,6 +2363,9 @@ export namespace vtadmin {
 
         /** GetSchemaTableSizeOptions aggregate_sizes */
         aggregate_sizes?: (boolean|null);
+
+        /** GetSchemaTableSizeOptions include_non_serving_shards */
+        include_non_serving_shards?: (boolean|null);
     }
 
     /** Represents a GetSchemaTableSizeOptions. */
@@ -2377,6 +2380,9 @@ export namespace vtadmin {
         /** GetSchemaTableSizeOptions aggregate_sizes. */
         public aggregate_sizes: boolean;
 
+        /** GetSchemaTableSizeOptions include_non_serving_shards. */
+        public include_non_serving_shards: boolean;
+
         /**
          * Creates a new GetSchemaTableSizeOptions instance using the specified properties.
          * @param [properties] Properties to set

web/vtadmin/src/proto/vtadmin.js

@@ -5511,6 +5511,7 @@ $root.vtadmin = (function() {
          * @memberof vtadmin
          * @interface IGetSchemaTableSizeOptions
          * @property {boolean|null} [aggregate_sizes] GetSchemaTableSizeOptions aggregate_sizes
+         * @property {boolean|null} [include_non_serving_shards] GetSchemaTableSizeOptions include_non_serving_shards
          */
 
         /**
@@ -5536,6 +5537,14 @@ $root.vtadmin = (function() {
          */
         GetSchemaTableSizeOptions.prototype.aggregate_sizes = false;
 
+        /**
+         * GetSchemaTableSizeOptions include_non_serving_shards.
+         * @member {boolean} include_non_serving_shards
+         * @memberof vtadmin.GetSchemaTableSizeOptions
+         * @instance
+         */
+        GetSchemaTableSizeOptions.prototype.include_non_serving_shards = false;
+
         /**
          * Creates a new GetSchemaTableSizeOptions instance using the specified properties.
          * @function create
@@ -5562,6 +5571,8 @@ $root.vtadmin = (function() {
                 writer = $Writer.create();
             if (message.aggregate_sizes != null && Object.hasOwnProperty.call(message, "aggregate_sizes"))
                 writer.uint32(/* id 1, wireType 0 =*/8).bool(message.aggregate_sizes);
+            if (message.include_non_serving_shards != null && Object.hasOwnProperty.call(message, "include_non_serving_shards"))
+                writer.uint32(/* id 2, wireType 0 =*/16).bool(message.include_non_serving_shards);
             return writer;
         };
 
@@ -5599,6 +5610,9 @@ $root.vtadmin = (function() {
                 case 1:
                     message.aggregate_sizes = reader.bool();
                     break;
+                case 2:
+                    message.include_non_serving_shards = reader.bool();
+                    break;
                 default:
                     reader.skipType(tag & 7);
                     break;
@@ -5637,6 +5651,9 @@ $root.vtadmin = (function() {
             if (message.aggregate_sizes != null && message.hasOwnProperty("aggregate_sizes"))
                 if (typeof message.aggregate_sizes !== "boolean")
                     return "aggregate_sizes: boolean expected";
+            if (message.include_non_serving_shards != null && message.hasOwnProperty("include_non_serving_shards"))
+                if (typeof message.include_non_serving_shards !== "boolean")
+                    return "include_non_serving_shards: boolean expected";
             return null;
         };
 
@@ -5654,6 +5671,8 @@ $root.vtadmin = (function() {
             var message = new $root.vtadmin.GetSchemaTableSizeOptions();
             if (object.aggregate_sizes != null)
                 message.aggregate_sizes = Boolean(object.aggregate_sizes);
+            if (object.include_non_serving_shards != null)
+                message.include_non_serving_shards = Boolean(object.include_non_serving_shards);
             return message;
         };
 
@@ -5670,10 +5689,14 @@ $root.vtadmin = (function() {
             if (!options)
                 options = {};
             var object = {};
-            if (options.defaults)
+            if (options.defaults) {
                 object.aggregate_sizes = false;
+                object.include_non_serving_shards = false;
+            }
             if (message.aggregate_sizes != null && message.hasOwnProperty("aggregate_sizes"))
                 object.aggregate_sizes = message.aggregate_sizes;
+            if (message.include_non_serving_shards != null && message.hasOwnProperty("include_non_serving_shards"))
+                object.include_non_serving_shards = message.include_non_serving_shards;
             return object;
         };