From f6215574f2e569cd6181ab762242e2db25b08fa6 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Sat, 30 Jan 2016 16:51:12 -0800 Subject: [PATCH] *: kv range to return current revision This changes the behavior of KV's range and tx range to return current revision rather than range revision. This makes populating range response header easier. --- clientv3/integration/client_test.go | 111 ++++++++++++++++++++++++++++ integration/v3_grpc_test.go | 5 +- storage/kv.go | 4 + storage/kv_test.go | 4 +- storage/kvstore.go | 9 ++- 5 files changed, 123 insertions(+), 10 deletions(-) diff --git a/clientv3/integration/client_test.go b/clientv3/integration/client_test.go index 3198066a0..010629372 100644 --- a/clientv3/integration/client_test.go +++ b/clientv3/integration/client_test.go @@ -16,12 +16,14 @@ package integration import ( "bytes" + "reflect" "testing" "github.com/coreos/etcd/clientv3" "github.com/coreos/etcd/integration" "github.com/coreos/etcd/lease" "github.com/coreos/etcd/pkg/testutil" + "github.com/coreos/etcd/storage/storagepb" ) func TestKVPut(t *testing.T) { @@ -61,3 +63,112 @@ func TestKVPut(t *testing.T) { } } } + +func TestKVRange(t *testing.T) { + defer testutil.AfterTest(t) + + clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 3}) + defer clus.Terminate(t) + + kv := clientv3.NewKV(clus.RandClient()) + + keySet := []string{"a", "b", "c", "c", "c", "foo", "foo/abc", "fop"} + for i, key := range keySet { + if _, err := kv.Put(key, "", lease.NoLease); err != nil { + t.Fatalf("#%d: couldn't put %q (%v)", i, key, err) + } + } + resp, err := kv.Get(keySet[0], 0) + if err != nil { + t.Fatalf("couldn't get key (%v)", err) + } + wheader := resp.Header + + tests := []struct { + begin, end string + rev int64 + sortOption *clientv3.SortOption + + wantSet []*storagepb.KeyValue + }{ + // range first two + { + "a", "c", + 0, + nil, + + []*storagepb.KeyValue{ + {Key: []byte("a"), Value: nil, CreateRevision: 2, ModRevision: 2, Version: 1}, + {Key: []byte("b"), Value: nil, CreateRevision: 3, ModRevision: 3, Version: 1}, + }, + }, + // range all with rev + { + "a", "x", + 2, + nil, + + []*storagepb.KeyValue{ + {Key: []byte("a"), Value: nil, CreateRevision: 2, ModRevision: 2, Version: 1}, + }, + }, + // range all with SortByKey, SortAscend + { + "a", "x", + 0, + &clientv3.SortOption{Target: clientv3.SortByKey, Order: clientv3.SortAscend}, + + []*storagepb.KeyValue{ + {Key: []byte("a"), Value: nil, CreateRevision: 2, ModRevision: 2, Version: 1}, + {Key: []byte("b"), Value: nil, CreateRevision: 3, ModRevision: 3, Version: 1}, + {Key: []byte("c"), Value: nil, CreateRevision: 4, ModRevision: 6, Version: 3}, + {Key: []byte("foo"), Value: nil, CreateRevision: 7, ModRevision: 7, Version: 1}, + {Key: []byte("foo/abc"), Value: nil, CreateRevision: 8, ModRevision: 8, Version: 1}, + {Key: []byte("fop"), Value: nil, CreateRevision: 9, ModRevision: 9, Version: 1}, + }, + }, + // range all with SortByCreatedRev, SortDescend + { + "a", "x", + 0, + &clientv3.SortOption{Target: clientv3.SortByCreatedRev, Order: clientv3.SortDescend}, + + []*storagepb.KeyValue{ + {Key: []byte("fop"), Value: nil, CreateRevision: 9, ModRevision: 9, Version: 1}, + {Key: []byte("foo/abc"), Value: nil, CreateRevision: 8, ModRevision: 8, Version: 1}, + {Key: []byte("foo"), Value: nil, CreateRevision: 7, ModRevision: 7, Version: 1}, + {Key: []byte("c"), Value: nil, CreateRevision: 4, ModRevision: 6, Version: 3}, + {Key: []byte("b"), Value: nil, CreateRevision: 3, ModRevision: 3, Version: 1}, + {Key: []byte("a"), Value: nil, CreateRevision: 2, ModRevision: 2, Version: 1}, + }, + }, + // range all with SortByModifiedRev, SortDescend + { + "a", "x", + 0, + &clientv3.SortOption{Target: clientv3.SortByModifiedRev, Order: clientv3.SortDescend}, + + []*storagepb.KeyValue{ + {Key: []byte("fop"), Value: nil, CreateRevision: 9, ModRevision: 9, Version: 1}, + {Key: []byte("foo/abc"), Value: nil, CreateRevision: 8, ModRevision: 8, Version: 1}, + {Key: []byte("foo"), Value: nil, CreateRevision: 7, ModRevision: 7, Version: 1}, + {Key: []byte("c"), Value: nil, CreateRevision: 4, ModRevision: 6, Version: 3}, + {Key: []byte("b"), Value: nil, CreateRevision: 3, ModRevision: 3, Version: 1}, + {Key: []byte("a"), Value: nil, CreateRevision: 2, ModRevision: 2, Version: 1}, + }, + }, + } + + for i, tt := range tests { + resp, err := kv.Range(tt.begin, tt.end, 0, tt.rev, tt.sortOption) + if err != nil { + t.Fatalf("#%d: couldn't range (%v)", i, err) + } + if !reflect.DeepEqual(wheader, resp.Header) { + t.Fatalf("#%d: wheader expected %+v, got %+v", i, wheader, resp.Header) + } + if !reflect.DeepEqual(tt.wantSet, resp.Kvs) { + t.Fatalf("#%d: resp.Kvs expected %+v, got %+v", i, tt.wantSet, resp.Kvs) + } + } +} diff --git a/integration/v3_grpc_test.go b/integration/v3_grpc_test.go index bcbc2d9f2..04ba159b0 100644 --- a/integration/v3_grpc_test.go +++ b/integration/v3_grpc_test.go @@ -1188,10 +1188,7 @@ func TestV3RangeRequest(t *testing.T) { if resp.More != tt.wmores[j] { t.Errorf("#%d.%d: bad more. got = %v, want = %v, ", i, j, resp.More, tt.wmores[j]) } - wrev := req.Revision - if wrev == 0 { - wrev = int64(len(tt.putKeys) + 1) - } + 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) } diff --git a/storage/kv.go b/storage/kv.go index ffdb43c69..a68e12060 100644 --- a/storage/kv.go +++ b/storage/kv.go @@ -30,6 +30,7 @@ type KV interface { FirstRev() int64 // Range gets the keys in the range at rangeRev. + // The returned rev is the current revision of the KV when the operation is executed. // If rangeRev <=0, range gets the keys at currentRev. // If `end` is nil, the request returns the key. // If `end` is not nil, it gets the keys in range [key, range_end). @@ -41,11 +42,13 @@ type KV interface { // attach a lease to a key-value pair as meta-data. KV implementation does not validate the lease // id. // A put also increases the rev of the store, and generates one event in the event history. + // The returned rev is the current revision of the KV when the operation is executed. Put(key, value []byte, lease lease.LeaseID) (rev int64) // DeleteRange deletes the given range from the store. // A deleteRange increases the rev of the store if any key in the range exists. // The number of key deleted will be returned. + // The returned rev is the current revision of the KV when the operation is executed. // It also generates one event for each key delete in the event history. // if the `end` is nil, deleteRange deletes the key. // if the `end` is not nil, deleteRange deletes the keys in range [key, range_end). @@ -58,6 +61,7 @@ type KV interface { TxnBegin() int64 // TxnEnd ends the on-going txn with txn ID. If the on-going txn ID is not matched, error is returned. TxnEnd(txnID int64) error + // TxnRange returns the current revision of the KV when the operation is executed. TxnRange(txnID int64, key, end []byte, limit, rangeRev int64) (kvs []storagepb.KeyValue, rev int64, err error) TxnPut(txnID int64, key, value []byte, lease lease.LeaseID) (rev int64, err error) TxnDeleteRange(txnID int64, key, end []byte) (n, rev int64, err error) diff --git a/storage/kv_test.go b/storage/kv_test.go index 7d29321ec..89f2b2863 100644 --- a/storage/kv_test.go +++ b/storage/kv_test.go @@ -153,8 +153,8 @@ func testKVRangeRev(t *testing.T, f rangeFunc) { }{ {-1, 4, kvs}, {0, 4, kvs}, - {2, 2, kvs[:1]}, - {3, 3, kvs[:2]}, + {2, 4, kvs[:1]}, + {3, 4, kvs[:2]}, {4, 4, kvs}, } diff --git a/storage/kvstore.go b/storage/kvstore.go index 27325ea12..16f8d15fd 100644 --- a/storage/kvstore.go +++ b/storage/kvstore.go @@ -353,8 +353,8 @@ func (a *store) Equal(b *store) bool { } // range is a keyword in Go, add Keys suffix. -func (s *store) rangeKeys(key, end []byte, limit, rangeRev int64) (kvs []storagepb.KeyValue, rev int64, err error) { - curRev := int64(s.currentRev.main) +func (s *store) rangeKeys(key, end []byte, limit, rangeRev int64) (kvs []storagepb.KeyValue, curRev int64, err error) { + curRev = int64(s.currentRev.main) if s.currentRev.sub > 0 { curRev += 1 } @@ -362,6 +362,7 @@ func (s *store) rangeKeys(key, end []byte, limit, rangeRev int64) (kvs []storage if rangeRev > curRev { return nil, s.currentRev.main, ErrFutureRev } + var rev int64 if rangeRev <= 0 { rev = curRev } else { @@ -373,7 +374,7 @@ func (s *store) rangeKeys(key, end []byte, limit, rangeRev int64) (kvs []storage _, revpairs := s.kvindex.Range(key, end, int64(rev)) if len(revpairs) == 0 { - return nil, rev, nil + return nil, curRev, nil } for _, revpair := range revpairs { @@ -393,7 +394,7 @@ func (s *store) rangeKeys(key, end []byte, limit, rangeRev int64) (kvs []storage break } } - return kvs, rev, nil + return kvs, curRev, nil } func (s *store) put(key, value []byte, leaseID lease.LeaseID) {