Adding optional revision bump to snapshot restore #16029
Conversation
dusk125
force-pushed
the
revision-bump
branch
5 times, most recently
from
9cd9656
to
64cefc6
Compare
|
To clarify by |
|
With this latest update, I've changed the revision-bump flag to only increase the latest revision by the amount so that clients shouldn't see decreasing revisions. |
etcdutl/snapshot/v3_snapshot.go
Outdated
| ) | ||
|
|
||
| latest.main += amount | ||
| k := [17]byte{} |
There was a problem hiding this comment.
Partially agree with @tjungblu . We should consider moving all revision related common functions (i.e. revToBytes, bytesToRev, newRevBytes) into package pkg (or api), so that they can be reused.
In this case, we should try to reuse newRevBytes. Please raise a followup ticket.
Signed-off-by: Allen Ray <alray@redhat.com>
|
I think that test failure may be a flake, running locally they succeeded. Can someone relaunch that test please? |
| return latest | ||
| } | ||
|
|
||
| func (s *v3Manager) unsafeMarkRevisionCompacted(tx backend.BatchTx, latest revision) { |
There was a problem hiding this comment.
Optional nit: it might be friendly if https://github.com/etcd-io/etcd/blob/main/server/storage/mvcc/kvstore.go#L201 can have a comment like "similar logic can be found in etcdctl side, this should be synced with the command"? Currently they share UnsafeSetScheduledCompact so it's not easy to make them diverged. If we add additional method calls to server side, the comment will be helpful for not forgetting about the etcdutl side logic (such change might be quite unlikely though).
|
Two left actions:
|
Tickets have been raised. I'll be on holiday for the next two weeks, so I can handle them when I get back (July 17th). |
|
While @dusk125 is out, I can take over the 3.5 backport. Can anyone hit the merge button? Thanks. |
Signed-off-by: Wenjia Zhang <wenjiazhang@google.com>
Signed-off-by: Wenjia Zhang <wenjiazhang@google.com>
Signed-off-by: Wenjia Zhang <wenjiazhang@google.com>
This PR adds the flags
--bump-revisionand--mark-compactedto etcdutl snapshot restore for #16028.I chose to handle this after the database is loaded from the snapshot, so as to not modify the snapshot, I didn't want to risk corrupting the snapshot as a part of this optional step. If for some reason the bump fails, a user would still be able to restore the snapshot again without the bump and restart the watchers manually.