diff --git a/etcdctl/ctlv3/ctl.go b/etcdctl/ctlv3/ctl.go index 4f9c31055c0..41d4ffbdbf3 100644 --- a/etcdctl/ctlv3/ctl.go +++ b/etcdctl/ctlv3/ctl.go @@ -52,7 +52,8 @@ func init() { rootCmd.PersistentFlags().StringSliceVar(&globalFlags.Endpoints, "endpoints", []string{"127.0.0.1:2379"}, "gRPC endpoints") rootCmd.PersistentFlags().BoolVar(&globalFlags.Debug, "debug", false, "enable client-side debug logging") - rootCmd.PersistentFlags().StringVarP(&globalFlags.OutputFormat, "write-out", "w", "simple", "set the output format (fields, json, protobuf, simple, table)") + rootCmd.PersistentFlags().StringVarP(&globalFlags.OutputFormat, "write-out", "w", "simple", + "set the output format (fields, json, protobuf, simple, table); note json encodes kvs as base64") rootCmd.PersistentFlags().BoolVar(&globalFlags.IsHex, "hex", false, "print byte strings as hex encoded strings") rootCmd.RegisterFlagCompletionFunc("write-out", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { return []string{"fields", "json", "protobuf", "simple", "table"}, cobra.ShellCompDirectiveDefault diff --git a/etcdutl/ctl.go b/etcdutl/ctl.go index e4106634744..c4a3d2fe6f7 100644 --- a/etcdutl/ctl.go +++ b/etcdutl/ctl.go @@ -35,7 +35,8 @@ var ( ) func init() { - rootCmd.PersistentFlags().StringVarP(&etcdutl.OutputFormat, "write-out", "w", "simple", "set the output format (fields, json, protobuf, simple, table)") + rootCmd.PersistentFlags().StringVarP(&etcdutl.OutputFormat, "write-out", "w", "simple", + "set the output format (fields, json, protobuf, simple, table); note json encodes kvs as base64") rootCmd.RegisterFlagCompletionFunc("write-out", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { return []string{"fields", "json", "protobuf", "simple", "table"}, cobra.ShellCompDirectiveDefault }) diff --git a/server/etcdserver/txn/txn.go b/server/etcdserver/txn/txn.go index 8f0e6c4b4a7..82b29cad35b 100644 --- a/server/etcdserver/txn/txn.go +++ b/server/etcdserver/txn/txn.go @@ -152,9 +152,6 @@ func Range(ctx context.Context, lg *zap.Logger, kv mvcc.KV, r *pb.RangeRequest) func executeRange(ctx context.Context, lg *zap.Logger, txnRead mvcc.TxnRead, r *pb.RangeRequest) (*pb.RangeResponse, error) { trace := traceutil.Get(ctx) - resp := &pb.RangeResponse{} - resp.Header = &pb.ResponseHeader{} - limit := r.Limit if r.SortOrder != pb.RangeRequest_NONE || r.MinModRevision != 0 || r.MaxModRevision != 0 || @@ -167,17 +164,52 @@ func executeRange(ctx context.Context, lg *zap.Logger, txnRead mvcc.TxnRead, r * limit = limit + 1 } + // Combining r.CountOnly and any of { r.MaxModRevision, r.MinModRevision, r.MaxCreateRevision, r.MinCreateRevision } + // means "give me the count for the result of the filtering I asked for". + filtering := r.MaxModRevision != 0 || r.MinModRevision != 0 || r.MaxCreateRevision != 0 || r.MinCreateRevision != 0 + var noKvsToProcess bool + var readLimit int64 + if filtering { + // if we are doing filtering, we can't limit our read; + // otherwise we can't calculate what the total count would be. + readLimit = 0 + noKvsToProcess = false + } else { + readLimit = limit + noKvsToProcess = r.CountOnly + } + ro := mvcc.RangeOptions{ - Limit: limit, + Limit: readLimit, Rev: r.Revision, - Count: r.CountOnly, + Count: noKvsToProcess, } rr, err := txnRead.Range(ctx, r.Key, mkGteRange(r.RangeEnd), ro) if err != nil { return nil, err } + resp := &pb.RangeResponse{Header: &pb.ResponseHeader{Revision: rr.Rev}} + + if noKvsToProcess { + resp.Count = int64(rr.Count) + resp.Kvs = make([]*mvccpb.KeyValue, 0) + } else { + processKvsInRange(r, rr, resp, trace, lg) + } + // No more pruning after this point; we can count now. + if filtering { + resp.Count = int64(len(rr.KVs)) + } else { + resp.Count = int64(rr.Count) + } + trace.Step("assemble the response") + return resp, nil +} + +func processKvsInRange(r *pb.RangeRequest, rr *mvcc.RangeResult, resp *pb.RangeResponse, trace *traceutil.Trace, lg *zap.Logger) { + trace.Step("filter and sort the key-value pairs") if r.MaxModRevision != 0 { f := func(kv *mvccpb.KeyValue) bool { return kv.ModRevision > r.MaxModRevision } pruneKVs(rr, f) @@ -195,6 +227,14 @@ func executeRange(ctx context.Context, lg *zap.Logger, txnRead mvcc.TxnRead, r * pruneKVs(rr, f) } + // If r.CountOnly was specified: + // * r.SortOrder is useless and ignored. + // * r.Limit is useless and ignored. + if r.CountOnly { + resp.Kvs = make([]*mvccpb.KeyValue, 0) + return + } + sortOrder := r.SortOrder if r.SortTarget != pb.RangeRequest_KEY && sortOrder == pb.RangeRequest_NONE { // Since current mvcc.Range implementation returns results @@ -235,9 +275,6 @@ func executeRange(ctx context.Context, lg *zap.Logger, txnRead mvcc.TxnRead, r * rr.KVs = rr.KVs[:r.Limit] resp.More = true } - trace.Step("filter and sort the key-value pairs") - resp.Header.Revision = rr.Rev - resp.Count = int64(rr.Count) resp.Kvs = make([]*mvccpb.KeyValue, len(rr.KVs)) for i := range rr.KVs { if r.KeysOnly { @@ -245,8 +282,6 @@ func executeRange(ctx context.Context, lg *zap.Logger, txnRead mvcc.TxnRead, r * } resp.Kvs[i] = &rr.KVs[i] } - trace.Step("assemble the response") - return resp, nil } func Txn(ctx context.Context, lg *zap.Logger, rt *pb.TxnRequest, txnModeWriteWithSharedBuffer bool, kv mvcc.KV, lessor lease.Lessor) (*pb.TxnResponse, *traceutil.Trace, error) { diff --git a/tests/e2e/ctl_v3_kv_test.go b/tests/e2e/ctl_v3_kv_test.go index 89d8412d0a9..0ffcfd8b95b 100644 --- a/tests/e2e/ctl_v3_kv_test.go +++ b/tests/e2e/ctl_v3_kv_test.go @@ -245,7 +245,7 @@ func getMinMaxCreateModRevTest(cx ctlCtx) { for i, tt := range tests { if err := ctlV3Get(cx, tt.args, tt.wkv...); err != nil { - cx.t.Errorf("getMinModRevTest #%d: ctlV3Get error (%v)", i, err) + cx.t.Errorf("getMinMaxCreateModRevTest #%d: ctlV3Get error (%v)", i, err) } } } @@ -387,15 +387,31 @@ type kv struct { key, val string } +// returns -1 if no string in ss with prefix is found, or the index of the first occurrence. +func findWithPrefix(ss []string, prefix string) int { + for i, s := range ss { + if strings.HasPrefix(s, prefix) { + return i + } + } + return -1 +} + func ctlV3Get(cx ctlCtx, args []string, kvs ...kv) error { - cmdArgs := append(cx.PrefixArgs(), "get") + cmdArgs := append(cx.PrefixArgs(), "get", "--write-out=fields") cmdArgs = append(cmdArgs, args...) if !cx.quorum { cmdArgs = append(cmdArgs, "--consistency", "s") } var lines []expect.ExpectedResponse for _, elem := range kvs { - lines = append(lines, expect.ExpectedResponse{Value: elem.key}, expect.ExpectedResponse{Value: elem.val}) + lines = append(lines, + expect.ExpectedResponse{Value: fmt.Sprintf("\"Key\" : \"%s\"", elem.key)}, + expect.ExpectedResponse{Value: fmt.Sprintf("\"Value\" : \"%s\"", elem.val)}, + ) + } + if findWithPrefix(args, "--limit") == -1 { + lines = append(lines, expect.ExpectedResponse{Value: fmt.Sprintf("\"Count\" : %d", len(kvs))}) } return e2e.SpawnWithExpects(cmdArgs, cx.envMap, lines...) } diff --git a/tests/integration/v3_grpc_test.go b/tests/integration/v3_grpc_test.go index 2cc4ab66032..a3c85a4ae63 100644 --- a/tests/integration/v3_grpc_test.go +++ b/tests/integration/v3_grpc_test.go @@ -1471,7 +1471,7 @@ func TestV3RangeRequest(t *testing.T) { {"rev2", "rev3", "rev4", "rev5", "rev6"}, }, []bool{false, false, false, false}, - []int64{5, 5, 5, 5}, + []int64{4, 2, 3, 5}, }, { "min/max create rev", @@ -1503,7 +1503,7 @@ func TestV3RangeRequest(t *testing.T) { {"rev2", "rev3", "rev6"}, }, []bool{false, false, false, false}, - []int64{3, 3, 3, 3}, + []int64{2, 2, 1, 3}, }, } @@ -1527,24 +1527,24 @@ func TestV3RangeRequest(t *testing.T) { continue } if len(resp.Kvs) != len(tt.wresps[j]) { - t.Errorf("#%d.%d: bad len(resp.Kvs). got = %d, want = %d, ", i, j, len(resp.Kvs), len(tt.wresps[j])) + t.Errorf("#%d.%d (%s): bad len(resp.Kvs). got = %d, want = %d, ", i, j, tt.name, len(resp.Kvs), len(tt.wresps[j])) continue } for k, wKey := range tt.wresps[j] { respKey := string(resp.Kvs[k].Key) if respKey != wKey { - t.Errorf("#%d.%d: key[%d]. got = %v, want = %v, ", i, j, k, respKey, wKey) + t.Errorf("#%d.%d (%s): key[%d]. got = %v, want = %v, ", i, j, tt.name, k, respKey, wKey) } } if resp.More != tt.wmores[j] { - t.Errorf("#%d.%d: bad more. got = %v, want = %v, ", i, j, resp.More, tt.wmores[j]) + t.Errorf("#%d.%d (%s): bad more. got = %v, want = %v, ", i, j, tt.name, resp.More, tt.wmores[j]) } if resp.GetCount() != tt.wcounts[j] { - t.Errorf("#%d.%d: bad count. got = %v, want = %v, ", i, j, resp.GetCount(), tt.wcounts[j]) + t.Errorf("#%d.%d (%s): bad count. got = %v, want = %v, ", i, j, tt.name, resp.GetCount(), tt.wcounts[j]) } wrev := int64(len(tt.putKeys) + 1) if resp.Header.Revision != wrev { - t.Errorf("#%d.%d: bad header revision. got = %d. want = %d", i, j, resp.Header.Revision, wrev) + t.Errorf("#%d.%d (%s): bad header revision. got = %d. want = %d", i, j, tt.name, resp.Header.Revision, wrev) } } })