*: add snapshot package #9118

gyuho · 2018-01-08T17:38:42Z

Fix #8632.
Address #9109 with tests in snapshot package.

codecov-io · 2018-01-08T21:31:01Z

Codecov Report

Merging #9118 into master will decrease coverage by 0.03%.
The diff coverage is 56.86%.

@@            Coverage Diff             @@
##           master    #9118      +/-   ##
==========================================
- Coverage   75.89%   75.86%   -0.04%     
==========================================
  Files         359      361       +2     
  Lines       30054    30153      +99     
==========================================
+ Hits        22810    22876      +66     
- Misses       5650     5678      +28     
- Partials     1594     1599       +5

Impacted Files	Coverage Δ
etcdctl/ctlv3/command/printer_table.go	`0% <0%> (ø)`	⬆️
etcdctl/ctlv3/command/printer_simple.go	`70.62% <0%> (ø)`	⬆️
etcdctl/ctlv3/command/printer.go	`43.26% <0%> (-0.55%)`	⬇️
etcdctl/ctlv3/command/printer_fields.go	`0% <0%> (ø)`	⬆️
snapshot/logger.go	`21.21% <21.21%> (ø)`
etcdctl/ctlv3/command/printer_json.go	`58.33% <33.33%> (ø)`	⬆️
etcdctl/ctlv3/command/snapshot_command.go	`67.32% <54.76%> (+1.98%)`	⬆️
snapshot/v3_snapshot.go	`64.7% <64.7%> (ø)`
etcdctl/ctlv2/command/watch_command.go	`57.5% <0%> (-22.5%)`	⬇️
pkg/adt/interval_tree.go	`88.28% <0%> (-4.21%)`	⬇️
... and 24 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 202cc9a...6fcee6b. Read the comment docs.

xiang90 · 2018-01-10T18:22:21Z

it seems that snapshot should be a sub pkg under etcdctl?

xiang90 · 2018-01-10T18:22:38Z

/cc @Quentin-M can you give it a look? is this what you want?

Quentin-M · 2018-01-10T21:59:16Z

That's awesome! Thank you so much!

xiang90 · 2018-01-19T21:18:33Z

snapshot/v3_snapshot.go

+	logger        grpclog.LoggerV2
+}
+
+func (s *v3Snapshotter) Save(ctx context.Context, cli *clientv3.Client, dbPath string) error {


the client should be part of the config i feel, not per save.

xiang90 · 2018-01-22T20:48:49Z

snapshot/logger.go

+)
+
+// DefaultLogger defines default snapshot logger based on "capnslog".
+var DefaultLogger grpclog.LoggerV2


this probably should not belong to snapshot pkg. it is not snapshot related.

xiang90 · 2018-01-22T20:49:41Z

snapshot/v3_snapshot.go

+)
+
+// Snapshotter defines snapshot methods.
+type Snapshotter interface {


rename to Manager? so it looks like snapshot.Manager.Save/Status/Restore

xiang90 · 2018-01-22T20:50:14Z

snapshot/v3_snapshot.go

+}
+
+// DBStatus is the snapshot file status.
+type DBStatus struct {


Just Status? Hash/Revision is not db (boltdb in our case) specific, but snapshot related.

xiang90 · 2018-01-22T20:51:30Z

snapshot/v3_snapshot.go

+}
+
+// Config defines snapshot config.
+type Config struct {


we probably should kill this config struct. this is not snapshot related configuration.

xiang90 · 2018-01-22T20:52:08Z

snapshot/v3_snapshot.go

+	return ds, nil
+}
+
+type revision struct {


move this to util.go?

xiang90 · 2018-01-22T20:52:27Z

snapshot/v3_snapshot.go

+
+// initIndex implements ConsistentIndexGetter so the snapshot won't block
+// the new raft instance by waiting for a future raft index.
+type initIndex int


move to util.go?

xiang90 · 2018-01-22T20:53:38Z

a few nits. look good to me in general.

gyuho · 2018-01-22T23:35:59Z

@xiang90 PTAL. All fixed.

xiang90 · 2018-01-23T00:08:53Z

lgtm once CI gets passed.

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>

gyuho requested a review from xiang90 January 8, 2018 18:27

gyuho force-pushed the snapshot-code branch 2 times, most recently from 2b0e785 to fe02424 Compare January 8, 2018 19:12

gyuho added the WIP label Jan 10, 2018

gyuho force-pushed the snapshot-code branch from fe02424 to 2ecb174 Compare January 10, 2018 22:55

gyuho removed the WIP label Jan 10, 2018

gyuho force-pushed the snapshot-code branch 2 times, most recently from b61117c to 5e7d66b Compare January 11, 2018 00:21

This was referenced Jan 16, 2018

Improve membership reconfiguration test coverage #9150

Closed

Improve snapshot backup/restore test coverage #9151

Closed

xiang90 reviewed Jan 19, 2018

View reviewed changes

gyuho added the WIP label Jan 19, 2018

gyuho force-pushed the snapshot-code branch from 5e7d66b to 439a170 Compare January 20, 2018 07:47

gyuho removed the WIP label Jan 22, 2018

gyuho force-pushed the snapshot-code branch from 439a170 to 6fcee6b Compare January 22, 2018 18:30

gyuho changed the title *: add snapshot package with restore tests *: add snapshot package Jan 22, 2018

gyuho added the kind/need-release-note-v3.4 label Jan 22, 2018

xiang90 reviewed Jan 22, 2018

View reviewed changes

snapshot/v3_snapshot.go Outdated

return ds, nil

}

type revision struct {

Copy link

Contributor

xiang90 Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this to util.go?

xiang90 reviewed Jan 22, 2018

View reviewed changes

gyuho added the WIP label Jan 22, 2018

gyuho mentioned this pull request Jan 22, 2018

pkg/logger: initial commit #9194

Merged

gyuho force-pushed the snapshot-code branch from 6fcee6b to c9bb115 Compare January 22, 2018 23:25

gyuho removed the WIP label Jan 22, 2018

gyuho added the WIP label Jan 23, 2018

gyuho force-pushed the snapshot-code branch from c9bb115 to cd82ff5 Compare January 23, 2018 00:44

gyuho removed the WIP label Jan 23, 2018

gyuho added 4 commits January 22, 2018 16:46

snapshot: initial commit

6e2555f

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>

e2e: use "snapshot.Status"

8b317df

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>

etcdctl/ctlv3: replace "dbStatus" with "snapshot.Status"

c3ba417

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>

etcdctl/ctlv3: use "snapshot" package for "snapshot" command

df689f4

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>

gyuho force-pushed the snapshot-code branch from cd82ff5 to df689f4 Compare January 23, 2018 00:46

gyuho merged commit 4a12eaf into etcd-io:master Jan 23, 2018

gyuho deleted the snapshot-code branch January 23, 2018 01:31

Quentin-M mentioned this pull request Feb 1, 2018

Implement snapshot restore using Go code rather than etcdctl Quentin-M/etcd-cloud-operator#2

Closed

luxas mentioned this pull request May 15, 2018

Better etcd snapshot/restore in the kubeadm upgrade logic kubernetes/kubeadm#618

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: add snapshot package #9118

*: add snapshot package #9118

gyuho commented Jan 8, 2018

codecov-io commented Jan 8, 2018 •

edited

xiang90 commented Jan 10, 2018

xiang90 commented Jan 10, 2018

Quentin-M commented Jan 10, 2018

xiang90 Jan 19, 2018

xiang90 Jan 22, 2018

xiang90 Jan 22, 2018

xiang90 Jan 22, 2018

xiang90 Jan 22, 2018

xiang90 Jan 22, 2018

xiang90 Jan 22, 2018

xiang90 commented Jan 22, 2018

gyuho commented Jan 22, 2018

xiang90 commented Jan 23, 2018

*: add snapshot package #9118

*: add snapshot package #9118

Conversation

gyuho commented Jan 8, 2018

codecov-io commented Jan 8, 2018 • edited

Codecov Report

xiang90 commented Jan 10, 2018

xiang90 commented Jan 10, 2018

Quentin-M commented Jan 10, 2018

xiang90 Jan 19, 2018

Choose a reason for hiding this comment

xiang90 Jan 22, 2018

Choose a reason for hiding this comment

xiang90 Jan 22, 2018

Choose a reason for hiding this comment

xiang90 Jan 22, 2018

Choose a reason for hiding this comment

xiang90 Jan 22, 2018

Choose a reason for hiding this comment

xiang90 Jan 22, 2018

Choose a reason for hiding this comment

xiang90 Jan 22, 2018

Choose a reason for hiding this comment

xiang90 commented Jan 22, 2018

gyuho commented Jan 22, 2018

xiang90 commented Jan 23, 2018

codecov-io commented Jan 8, 2018 •

edited