api: hash by rev #8263

fanminshi · 2017-07-14T00:45:43Z

xiang90 · 2017-07-14T00:52:58Z

mvcc/backend/backend.go

@@ -52,6 +52,7 @@ type Backend interface {

 	Snapshot() Snapshot
 	Hash(ignores map[IgnoreKey]struct{}) (uint32, error)
+	HashByRev(bucket []byte, f func(rev []byte) bool) (uint32, error)


HashBucket? Backend should know nothing about revision.

xiang90 · 2017-07-14T00:54:24Z

mvcc/kv.go

@@ -111,6 +111,9 @@ type KV interface {
 	// This method is designed for consistency checking purposes.
 	Hash() (hash uint32, revision int64, err error)

+	// HashByRev retrieves the hash of KV state given a revision.
+	HashByRev(rev int64) (hash uint32, compactRev int64, err error)


Hash includes all state (including leases for example). HashByRev only hashes the kv state for good reason (lease does not understand rev yet...). we need to make this clear though.

heyitsanthony · 2017-07-14T04:28:28Z

mvcc/kvstore.go

+		return 0, 0, ErrFutureRev
+	}
+
+	compactc := s.compactc


Avoid holding the lock while waiting on compaction?

for { s.mu.RLock() compactMainRev := s.compactMainRev currentRev := s.currentRev compactc := s.compactc s.mu.RUnlock() if rev < s.compactMainRev { return 0, compactMainRev, ErrCompacted } else if rev > s.currentRev { return 0, compactMainRev, ErrFutureRev } select { case <-compactc: case <-s.stopc: return 0, 0, ErrClosed } s.mu.Lock() if s.compactc == compactc { defer s.mu.Unlock() break } s.mu.Unlock() } s.b.ForceCommit()

sounds good.

fanminshi · 2017-07-14T19:47:19Z

all fixed. PTAL

xiang90 · 2017-07-14T19:49:03Z

mvcc/backend/backend.go

@@ -240,6 +241,37 @@ func (b *backend) Hash(ignores map[IgnoreKey]struct{}) (uint32, error) {
 	return h.Sum32(), nil
 }

+func (b *backend) HashBucket(bucket []byte, f func(rev []byte) bool) (hash uint32, err error) {


explain what the pass in func does.

xiang90 · 2017-07-14T19:49:27Z

we need some tests for this.

heyitsanthony · 2017-07-14T19:50:38Z

mvcc/kvstore.go

@@ -42,8 +42,12 @@ var (
 	ErrFutureRev = errors.New("mvcc: required revision is a future revision")
 	ErrCanceled  = errors.New("mvcc: watcher is canceled")
 	ErrClosed    = errors.New("mvcc: closed")
+	ErrTimeout   = errors.New("mvcc: req hash timeout")


heyitsanthony · 2017-07-14T19:50:39Z

mvcc/kvstore.go


 	plog = capnslog.NewPackageLogger("github.com/coreos/etcd", "mvcc")
+
+	// hashTimeout is the max waiting time on the ongoing compaction.
+	hashTimeout = time.Duration(10) * time.Second


heyitsanthony · 2017-07-14T19:52:46Z

mvcc/kv.go

@@ -107,10 +107,14 @@ type KV interface {
 	// Write creates a write transaction.
 	Write() TxnWrite

-	// Hash retrieves the hash of KV state and revision.
-	// This method is designed for consistency checking purposes.
+	// Hash retrieves the hash of all states including leases.


this can stay the same?

Is KV state here means the entire Key-Value space of db which includes leases?

maybe Hash computes the hash of the KV's backend, then?

sounds better.

heyitsanthony · 2017-07-14T19:53:33Z

mvcc/kv.go

 	Hash() (hash uint32, revision int64, err error)

+	// HashByRev retrieves the hash of the KV state at a revision.


// HashByRev computes the hash of all MVCC keys up to a given revision.

heyitsanthony · 2017-07-14T19:54:01Z

mvcc/kv.go

 	Hash() (hash uint32, revision int64, err error)

+	// HashByRev retrieves the hash of the KV state at a revision.
+	// This method is designed to check consistency of the KV space.


probably don't need to explain what the hash is being used for

heyitsanthony · 2017-07-14T19:56:12Z

etcdserver/api/v3rpc/maintenance.go

@@ -128,11 +128,11 @@ func (ms *maintenanceServer) Snapshot(sr *pb.SnapshotRequest, srv pb.Maintenance
 }

 func (ms *maintenanceServer) Hash(ctx context.Context, r *pb.HashRequest) (*pb.HashResponse, error) {
-	h, rev, err := ms.kg.KV().Hash()


still need to call Hash() if r.Revision == 0, otherwise the functional tester won't check all the buckets

sounds good.

xiang90 · 2017-07-14T19:56:56Z

etcdserver/api/v3rpc/maintenance.go

@@ -128,11 +128,11 @@ func (ms *maintenanceServer) Snapshot(sr *pb.SnapshotRequest, srv pb.Maintenance
 }

 func (ms *maintenanceServer) Hash(ctx context.Context, r *pb.HashRequest) (*pb.HashResponse, error) {


not sure if we should change this.

the full hash is still useful.

we might want to add a quickHash which only cares about kv space and can do this incrementally.

I was thinking it could be rev == 0 => full hash?

is quick hash a flag in HashRequest or another api called quickHash?

I went with @heyitsanthony suggestion. one issue with current full hash operation it ignores compaction. However, I can probably merge Hash() into HashByRev () in to one function so both s.b.Hash and s.b.HashBucket will wait for compaction.

something like following

func (s *store) HashByRev(rev int64) (hash uint32, currentRev int64, compactRev int64, err error) { for { s.mu.RLock() compactMainRev := s.compactMainRev currentRev = s.currentRev compactc := s.compactc s.mu.RUnlock() if rev > 0 && rev < compactMainRev { return 0, 0, compactMainRev, ErrCompacted } else if rev > 0 && rev > currentRev { return 0, currentRev, 0, ErrFutureRev } select { case <-compactc: case <-s.stopc: return 0, 0, 0, ErrClosed } s.mu.Lock() if s.compactc == compactc { defer s.mu.Unlock() break } s.mu.Unlock() } s.b.ForceCommit() if rev == 0 { // full hash if rev == 0 hash, err = s.b.Hash(DefaultIgnores) } else { upper := revision{main: rev + 1} f := func(r []byte) bool { return upper.GreaterThan(bytesToRev(r)) } hash, err = s.b.HashBucket(keyBucketName, f) } return hash, s.currentRev, s.compactMainRev, err }

that makes sense.

Maybe there should be a separate RPC (HashRev? HashKV?), then; there might be cases where hashing the keys bucket with the current revision is desirable and there's no way to do that when reusing the current Hash RPC without having a flag like quickHash, which seems kind of hacky.

HashKV sounds good to me.
Should HashKV has its own req/resp type? or should it just use HashRequest and HashResponse defined in this pr?

It should take HashKVRequest and return a HashKVResponse

probably a different request type/resp makes more sense since Maintenance Hash API shouldn't have reason about rev in HashRequest nor compact_revision in HashResponse

heyitsanthony · 2017-07-14T20:02:58Z

If this is going to be used for background consistency checking, it should probably use ReadTx. Right now it seems this will block all mvcc operations while scanning the keys bucket? The current approach is OK as a first cut, though.

fanminshi · 2017-07-14T20:18:59Z

mvcc/kvstore.go

+	for {
+		s.mu.RLock()
+		compactMainRev := s.compactMainRev
+		currentRev := s.currentRev


@heyitsanthony should HashByRev also return currentRev to maintenance api where it will be used as the rev in the header?

should the rev be the kv state when hash finishes?

e.g return hash, s.currentRev, s.compactMainRev, err

The revision at the time of starting the hash. It'll matter more once it's nonblocking. Consider:

Start Hash(rev=10); currentRev=100
Compact(rev=50); currentRev=100
Put; currentRev=101
Finish Hash; currentRev=101

If Hash returns 101 as its header revision instead of 100, then etcd is saying the Hash would have worked at rev=101. This cannot be true since the keys for the hash at rev=10 have been compacted away by rev=101.

This is similar to the reasoning for Range's header revisions. If I request a Get with rev=0, then I'd expect the same KV values (or a compaction error) if I make a request later using WithRev(getResp.Header.Revision). This is guaranteed if the header is populated with the revision at the time of starting the read transaction, but not if it's using the revision after the transaction completes.

that's right.

fanminshi · 2017-07-14T20:21:20Z

mvcc/kvstore.go

+		}
+		s.mu.Lock()
+		if s.compactc == compactc {
+			defer s.mu.Unlock()


probably need to retrieve currentRev = s.currentRev here because it is under the s.mu.Lock() which marks the beginning KV state of HashBucket operation.

anything greater than the rev argument will be ignored; I don't think it necessarily needs the most up to date current revision, just one that is >= rev

xiang90 · 2017-07-14T21:25:20Z

it should probably use ReadTx.

If we use ReadTx, it can only see the top layer of KV, but not the historical versions, right? I am not sure how are we going to calculate the hash incrementally with that approach. Or we need to make sure ReadTx is really cheap and will not block anything, and calculating the hash of a few mill keys is also cheap enough.

heyitsanthony · 2017-07-14T21:31:35Z

@xiang90 ReadTx can efficiently scan historical data; it's keyed by revision on the mvcc bucket just like batchtx. Maybe you're thinking of mvcc.TxnRead? It also has the advantage that it won't need to force commit like when scanning with db.View.

What do you mean by computing the hash incrementally?

xiang90 · 2017-07-14T21:39:59Z

Maybe you're thinking of mvcc.TxnRead?

yea. i was thinking of the readView thing. sorry for the confusion.

HashKV rpc hash of all MVCC keys up to a given revision for a given node.

fanminshi · 2017-07-14T23:48:17Z

add HashKV and use backend.ReadTx to traverse Key-Value space. PTAL

codecov-io · 2017-07-15T01:03:38Z

Codecov Report

Merging #8263 into master will decrease coverage by 0.09%.
The diff coverage is 19.64%.

@@           Coverage Diff            @@
##           master   #8263     +/-   ##
========================================
- Coverage    76.2%   76.1%   -0.1%     
========================================
  Files         346     346             
  Lines       27055   27110     +55     
========================================
+ Hits        20616   20632     +16     
- Misses       4957    4992     +35     
- Partials     1482    1486      +4

Impacted Files	Coverage Δ
mvcc/kv.go	`40% <ø> (ø)`	⬆️
etcdserver/api/v3rpc/maintenance.go	`69.07% <0%> (-8.84%)`	⬇️
proxy/grpcproxy/maintenance.go	`42.85% <0%> (-4.02%)`	⬇️
...xy/grpcproxy/adapter/maintenance_client_adapter.go	`30.3% <0%> (-1.96%)`	⬇️
mvcc/kvstore_txn.go	`86.01% <100%> (+0.19%)`	⬆️
mvcc/kvstore.go	`81.1% <21.62%> (-10.19%)`	⬇️
etcdserver/api/v3rpc/lease.go	`89.28% <0%> (-7.15%)`	⬇️
clientv3/namespace/watch.go	`87.87% <0%> (-6.07%)`	⬇️
pkg/testutil/recorder.go	`77.77% <0%> (-3.71%)`	⬇️
lease/leasehttp/http.go	`61.06% <0%> (-3.06%)`	⬇️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfbf666...d06f69b. Read the comment docs.

xiang90 · 2017-07-16T22:01:16Z

mvcc/kv.go

 	Hash() (hash uint32, revision int64, err error)

+	// HashByRev computes the hash of all MVCC keys up to a given revision.


the hash of only mvcc keys up to a given revision (excluding KV backend metadata, leases, etc.).

i wonder if there is a keyword represents just key-pair values of the "key" bucket?

computes the hash of all MVCC revisions up to a given revision? that would cover deleted keys as well

heyitsanthony · 2017-07-17T16:58:23Z

mvcc/kvstore.go

@@ -160,6 +166,56 @@ func (s *store) Hash() (hash uint32, revision int64, err error) {
 	return h, s.currentRev, err
 }

+func (s *store) HashByRev(rev int64) (hash uint32, currentRev int64, compactRev int64, err error) {
+	readTx := s.b.ReadTx()
+	for {


this isn't needed with readtx, it should be enough to have TxnRead:
kvstore_txn.go:

func (s *store) Read() TxnRead { return newMetricsTxnRead(s.read()) } func (s *store) read() *storeTxnRead { s.mu.RLock() tx := s.b.ReadTx() s.revMu.RLock() tx.Lock() firstRev, rev := s.compactMainRev, s.currentRev s.revMu.RUnlock() return &storeTxnRead{s, tx, firstRev, rev} }

func (s *store) HashByRev(rev int64) (int32, int64, int64) { txnRead := s.read() defer txnRead.End() ... txnRead.tx.UnsafeForEach(keyBucketName, func(k, v []byte) error { ... } ... return hash, readTxn.Rev(), readTxn.FirstRev() }

I wonder is waiting for compaction even necessary?
The moment i obtain the txnRead I have the view of the world; i know the compaction rev, current rev.

Then could I just hash from all the ways between [compaction rev, current rev]?

there's no need to wait on compaction with txnread; I should have made that more clear...

one more thought. if I want to hash all the ways between [compaction rev, current rev], I can't simply just hash all the key that has rev > compaction rev and <= current rev because compaction deletes historic revisions for a key if it is not tombstone. In that case, I need to skip key that's historic and has rev < compaction rev. hence, I need a way to determine if a key is historic. How would that be achieve easily just by looping through kv space?

It seems to me that waiting for the compaction to finish can be a simpler approach where i don't have to worry about checking tombstone and historic keys that has rev < compaction rev.

it shouldn't skip historic keys; can you write a test where the hashing fails because of the problem you're describing?

@heyitsanthony I wrote a test to illustrate my concern.

func TestHashBeforeCompactionFinishes(t *testing.T) { defer testutil.AfterTest(t) clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 3}) defer clus.Terminate(t) cli.Put(context.TODO(), "foo", "bar2") // rev = 2 cli.Put(context.TODO(), "foo", "bar3") // rev = 3 cli.Put(context.TODO(), "bla", "bar4") // rev = 4 cli := clus.Client(0) cli1 := clus.Client(1) // compact starting cli.Compact(context.TODO(), 3) // suppose that ongoing compaction on node 0 hasn't delete anything yet. // if HashKV doesn't skip historic keys of "foo" that's before the compact rev 3,, then it will return a hash // that's composed of ["foo", "bar2"], ["foo", "bar3"], ["bla", "bar4"]. // If hashKV skips historic keys of "foo", then it needs to figure out which // key rev will be compacted; skip ["foo", "bar2"] since it will be compacted and // return hash ["foo", "bar3"] and ["bla", "bar4"]. resp, _ = toGRPC(cli).Maintainance.HashKV(context.TODO(), &HashKVRequest{Revision: 4}) // suppose that ongoing compaction on node 1 completes before this Hash begins, // then it will return a hash consists only ["foo", "bar3"] and ["bla", "bar4"] // regardless skipping historic keys or not. resp2, _ = toGRPC(cli2).Maintainance.HashKV(context.TODO(), &HashKVRequest{Revision: 4}) // expect resp.CompactRevision == resp2.CompactRevision // and resp.Hash != resp2.Hash in the scenario no skipping historic keys. if resp.CompactRevisions == resp2.CompactRevision { if resp.Hash != resp2.CompactRevision { t.Fatalf("hash mismatch on same CompactRevision") } } }

Two possible solutions:

open the read txn when there's no compaction scheduled (pricey since it will have to forcecommit, but simpler)

hash all revisions >=compactRev, then scan the key index to get keys at the current rev with modrev < compactRev.

There should be a test to trigger this inflight behavior in the mvcc package. It doesn't look like that integration test will hit the problem very often...

yeah, the test is just to illustrate the issue; a better test is need.

I'll give it a try on the second approach.

heyitsanthony · 2017-07-17T18:03:07Z

test case?

fanminshi · 2017-07-17T18:08:20Z

will add tests.

fanminshi · 2017-07-20T21:53:43Z

@heyitsanthony you are right.

fanminshi · 2017-07-21T21:21:35Z

add test and implement compaction wait free HashKV. PTAL.

heyitsanthony · 2017-07-21T21:25:51Z

mvcc/kvstore.go

@@ -44,6 +45,8 @@ var (
 	ErrClosed    = errors.New("mvcc: closed")

 	plog = capnslog.NewPackageLogger("github.com/coreos/etcd", "mvcc")
+
+	EmptyKeep = make(map[revision]struct{})


probably can be now used for make(struct map[rev]struct{} in your comment below

"If there are no scheduled compactions, won't this always force a commit to disk? Isn't that something keep is trying to avoid? a possible workaround is to set s.keep = make(struct map[rev]struct{}) after this ForceCommit (and have len(keep) > 0 instead of keep != nil in UnsafeForEach) so it won't commit on every call."

emptyKeep; shouldn't be exported

heyitsanthony · 2017-07-21T21:31:53Z

mvcc/kvstore.go

+	if keep == nil {
+		// ForceCommit ensures that txnRead begins after backend
+		// has committed all the changes from the prev completed compaction.
+		s.b.ForceCommit()


If there are no scheduled compactions, won't this always force a commit to disk? Isn't that something keep is trying to avoid? a possible workaround is to set s.keep = make(struct map[rev]struct{}) after this ForceCommit (and have len(keep) > 0 instead of keep != nil in UnsafeForEach) so it won't commit on every call.

having an empty keep make sense.

heyitsanthony · 2017-07-21T21:33:26Z

mvcc/kvstore.go

+		tx   backend.ReadTx
+	)
+	s.mu.Lock()
+	s.revMu.Lock()


heyitsanthony · 2017-07-21T21:33:33Z

mvcc/kvstore.go

+	compactRev, currentRev = s.compactMainRev, s.currentRev
+	s.revMu.Unlock()
+
+	s.keepMu.Lock()


heyitsanthony · 2017-07-21T21:33:41Z

mvcc/kvstore.go

+	s.revMu.Unlock()
+
+	s.keepMu.Lock()
+	keep = s.keep


keep := s.keep

heyitsanthony · 2017-07-21T21:52:27Z

integration/v3_grpc_inflight_test.go

+	cctx, cancel := context.WithCancel(context.Background())
+	var rev int64
+	for i := 0; i < 100; i++ {
+		resp, err := cli1.Put(cctx, "foo", "bar"+strconv.Itoa(i))


grpc integration tests should use raw RPCs instead of going through the client RPC wrappers

heyitsanthony · 2017-07-21T21:54:33Z

integration/v3_grpc_inflight_test.go

@@ -54,6 +55,101 @@ func TestV3MaintenanceDefragmentInflightRange(t *testing.T) {
 	<-donec
 }

+// TestV3CompactInflightHashKV ensures that HashKV returns
+// correct hash while compacting.
+func TestV3CompactInflightHashKV(t *testing.T) {


This sort of stress test may better belong in the mvcc package since if there's a concurrency problem it should turn up with a single kv instance. The grpc test could instead only test functionality instead of stressing it

I saw that mvcc pkg only have unit tests. In that case, should I just move the exact same test to some new test.go file in mvcc? or should I transform my test to some sort of unit test on kvstore?

The test in mvcc would use a kvstore to test/stress HashByRev; the test in integrations/ would test that the server responds to the RPC

heyitsanthony · 2017-07-21T22:05:16Z

mvcc/kvstore.go

 func (s *store) Compact(rev int64) (<-chan struct{}, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	s.revMu.Lock()
 	defer s.revMu.Unlock()
-
+	s.keepMu.Lock()


only hold lock when updating s.keep?

heyitsanthony · 2017-07-21T22:07:23Z

integration/v3_grpc_inflight_test.go

+	go func() {
+		defer close(donec)
+		revHash := make(map[int64]uint32)
+		for i := 0; i < 12; i++ {


if using the Compact loop goroutine, this should probably loop until at least there is one hit in revHash so there's always some hash comparison on the same compact rev

that's a good idea.

heyitsanthony · 2017-07-21T22:08:26Z

integration/v3_grpc_inflight_test.go

+
+				select {
+				case hashesc <- resp:
+				case <-donec:


case <-cctx.Done():?

fanminshi · 2017-07-22T00:10:54Z

All fixed. PTAL.

heyitsanthony · 2017-07-22T00:16:48Z

the tests still aren't in the mvcc package?

fanminshi · 2017-07-24T17:17:09Z

one quick question. Shall I have my test in mvcc/integration pkg in some test.go file?

heyitsanthony · 2017-07-24T17:34:40Z

@fanminshi there would be a new test in mvcc/kv_test.go that only tests the backend instead of issuing RPCs to stress the feature. The grpc integration test would only test that there's a reasonable response / the RPC is wired up right.

fanminshi · 2017-07-25T23:26:55Z

add a test in mvcc to test HashKV in kvstore only and a integration test to Integration pkg to test HashKV rpc.

heyitsanthony · 2017-07-25T23:31:49Z

mvcc/kvstore_test.go

+			if err != nil {
+				t.Fatal(err)
+			}
+			time.Sleep(10)


use n * time.Millisecond so the time resolution is clear

heyitsanthony · 2017-07-25T23:32:18Z

mvcc/kvstore_test.go

+			case r := <-hashCompactc:
+				if revHash[r.compactRev] == 0 {
+					revHash[r.compactRev] = r.hash
+					break


no need to break

heyitsanthony · 2017-07-25T23:33:16Z

mvcc/kvstore_test.go

+		s.Put([]byte("foo"), []byte("bar"+strconv.Itoa(i)), lease.NoLease)
+	}
+
+	hashCompactc := make(chan struct {


declare this struct above the func instead of inlining

type hashKVResult struct { hash uint32 compactRev int64 }

heyitsanthony · 2017-07-25T23:37:33Z

mvcc/kvstore_test.go

+		defer close(donec)
+		revHash := make(map[int64]uint32)
+		for round := 0; round < 1000; round++ {
+			select {


there's only one case in the select, so pull from the channel directly:

r := <-hashCompactc if revHash[r.compactRev] == 0 { ...

heyitsanthony · 2017-07-25T23:42:14Z

integration/v3_grpc_test.go

+	}
+
+	// ensure appliedIdx is sync with committedIdx
+	_, err := kvc.Range(context.Background(), &pb.RangeRequest{Key: []byte("foo")})


this isn't necessary for a single member cluster; there won't be an ack for the put until it's synced

heyitsanthony · 2017-07-25T23:44:23Z

integration/v3_grpc_test.go

+		if err != nil {
+			t.Fatal(err)
+		}
+		rev = resp.Header.Revision


why not HashKV after every put and check that there's no match?

heyitsanthony · 2017-07-25T23:47:29Z

integration/v3_grpc_test.go

+	}
+
+	var (
+		prevHash       uint32


can set these when looping on the puts and fetching the hash; avoids the need for the if prevHash == 0 check

heyitsanthony · 2017-07-25T23:47:49Z

integration/v3_grpc_test.go

+		prevCompactRev int64
+	)
+	for i := 0; i < 10; i++ {
+		resp, err := mvc.HashKV(context.Background(), &pb.HashKVRequest{rev})


use rev=0 => current rev with &pb.HashKVRequest{}?

heyitsanthony · 2017-07-25T23:49:33Z

mvcc/kvstore_test.go

+
+	rev := 1000
+	for i := 2; i <= rev; i++ {
+		s.Put([]byte("foo"), []byte("bar"+strconv.Itoa(i)), lease.NoLease)


fmt.Sprintf("bar%d", i) to avoid the strconv import

heyitsanthony · 2017-07-25T23:49:57Z

mvcc/kvstore.go

 func (s *store) Compact(rev int64) (<-chan struct{}, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	s.revMu.Lock()
 	defer s.revMu.Unlock()
-


keep old spacing?

HashByRev computes the hash of all MVCC keys up to a given revision.

fanminshi · 2017-07-26T01:17:45Z

All fixed. PTAL

heyitsanthony

lgtm thanks

fanminshi added the WIP label Jul 14, 2017

xiang90 reviewed Jul 14, 2017

View reviewed changes

heyitsanthony reviewed Jul 14, 2017

View reviewed changes

gyuho added the kind/release-note label Jul 14, 2017

fanminshi force-pushed the hash_by_rev branch 2 times, most recently from 7e852b3 to 614b3b1 Compare July 14, 2017 19:46

xiang90 reviewed Jul 14, 2017

View reviewed changes

fanminshi force-pushed the hash_by_rev branch from 614b3b1 to edc7a03 Compare July 14, 2017 19:54

heyitsanthony suggested changes Jul 14, 2017

View reviewed changes

xiang90 reviewed Jul 14, 2017

View reviewed changes

fanminshi commented Jul 14, 2017

View reviewed changes

fanminshi force-pushed the hash_by_rev branch from edc7a03 to 663dec1 Compare July 14, 2017 20:42

fanminshi added 3 commits July 14, 2017 16:41

proto: add HashKV grpc

ed052ce

HashKV rpc hash of all MVCC keys up to a given revision for a given node.

doc: update rpc swagger for HashKV rpc and its req/resp

3dcd2cd

v3rpc: add HashKV to server rpc

fbb75d2

fanminshi force-pushed the hash_by_rev branch from 663dec1 to 565e447 Compare July 14, 2017 23:46

xiang90 reviewed Jul 16, 2017

View reviewed changes

heyitsanthony reviewed Jul 17, 2017

View reviewed changes

fanminshi force-pushed the hash_by_rev branch from 565e447 to cd5e223 Compare July 20, 2017 00:04

fanminshi force-pushed the hash_by_rev branch 2 times, most recently from a800586 to 72366b8 Compare July 21, 2017 21:18

heyitsanthony suggested changes Jul 21, 2017

View reviewed changes

fanminshi removed the WIP label Jul 21, 2017

fanminshi force-pushed the hash_by_rev branch 3 times, most recently from 897a346 to 2062074 Compare July 22, 2017 00:09

fanminshi force-pushed the hash_by_rev branch from 2062074 to e83cc64 Compare July 25, 2017 23:24

fanminshi force-pushed the hash_by_rev branch 2 times, most recently from 6543444 to 767a217 Compare July 25, 2017 23:40

heyitsanthony suggested changes Jul 25, 2017

View reviewed changes

fanminshi added 2 commits July 25, 2017 17:00

mvcc: add HashByRev to kv.go

deca987

HashByRev computes the hash of all MVCC keys up to a given revision.

proxy: support HashKV in grpcproxy

a6ae677

fanminshi force-pushed the hash_by_rev branch from 767a217 to 1fa86aa Compare July 26, 2017 00:56

fanminshi added 2 commits July 26, 2017 09:48

integration: add TestV3HashKV in v3_grpc_test.go

766c254

mvcc: add TestHashKVWhenCompacting to kvstore_test

8609521

fanminshi force-pushed the hash_by_rev branch from 1fa86aa to 8609521 Compare July 26, 2017 16:48

heyitsanthony approved these changes Jul 26, 2017

View reviewed changes

xiang90 merged commit 2a348fb into etcd-io:master Jul 26, 2017

wenjiaswe mentioned this pull request Jul 20, 2018

Automated cherry pick of #9761 #9942

Merged

jmhbnz mentioned this pull request Jan 25, 2024

test: fix TestHashKVWhenCompacting: ensure all goroutine finished #17313

Merged

fuweid mentioned this pull request Jul 16, 2024

HashKV should compute the hash of all MVCC keys up to a given revision #18337

Open

4 tasks

		Hash() (hash uint32, revision int64, err error)

		// HashByRev retrieves the hash of the KV state at a revision.

		@@ -128,11 +128,11 @@ func (ms maintenanceServer) Snapshot(sr pb.SnapshotRequest, srv pb.Maintenance
		}

		func (ms maintenanceServer) Hash(ctx context.Context, r pb.HashRequest) (*pb.HashResponse, error) {

api: hash by rev #8263

api: hash by rev #8263

Conversation

fanminshi commented Jul 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heyitsanthony Jul 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fanminshi commented Jul 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiang90 commented Jul 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fanminshi Jul 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heyitsanthony Jul 14, 2017 • edited Loading

Choose a reason for hiding this comment

fanminshi Jul 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heyitsanthony commented Jul 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fanminshi Jul 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiang90 commented Jul 14, 2017

heyitsanthony commented Jul 14, 2017 • edited Loading

xiang90 commented Jul 14, 2017 • edited Loading

fanminshi commented Jul 14, 2017

codecov-io commented Jul 15, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heyitsanthony Jul 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heyitsanthony commented Jul 17, 2017

fanminshi commented Jul 17, 2017

fanminshi commented Jul 20, 2017

fanminshi commented Jul 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fanminshi Jul 21, 2017 • edited Loading

heyitsanthony Jul 14, 2017 •

edited

Loading

fanminshi Jul 14, 2017 •

edited

Loading

heyitsanthony Jul 14, 2017 •

edited

Loading

fanminshi Jul 14, 2017 •

edited

Loading

fanminshi Jul 14, 2017 •

edited

Loading

heyitsanthony commented Jul 14, 2017 •

edited

Loading

xiang90 commented Jul 14, 2017 •

edited

Loading

codecov-io commented Jul 15, 2017 •

edited

Loading

heyitsanthony Jul 18, 2017 •

edited

Loading

fanminshi Jul 21, 2017 •

edited

Loading

heyitsanthony Jul 21, 2017 •

edited

Loading