Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

etcdctl: write snapshot using storage backend #4979

Closed

Conversation

heyitsanthony
Copy link
Contributor

No description provided.

@gyuho
Copy link
Contributor

gyuho commented Apr 6, 2016

@heyitsanthony lgtm. Defer to @xiang90

And do we have Readme on this command? Can't find.

@heyitsanthony
Copy link
Contributor Author

@gyuho no readme for snapshot yet

@gyuho
Copy link
Contributor

gyuho commented Apr 6, 2016

Ok got it.

@xiang90
Copy link
Contributor

xiang90 commented Apr 6, 2016

What is the plan here for snapshot auth/leases?

@heyitsanthony
Copy link
Contributor Author

@xiang90 a few options:
a. drop leases/auth on the floor on snapshots
b. add list RPCs for leases/auth
c. have a generic way to list out raw bucketed data via internal RPC (reuse a subset of KV API; not full mvcc, not a snapshot)

(a) is a non-starter if we want to preserve long lived leases out of the gate, but it lets us defer support for leases/auth until a later date
(b) is straightforward but will be a lot of specialized code to do basically the same thing.
(c) is what I prefer since it seems like an opportunity for code reuse via a unified interface across services. This could be more effort than (b) but I suspect it will be less effort in the long run.

@xiang90
Copy link
Contributor

xiang90 commented Apr 6, 2016

@heyitsanthony I feel c) is better.

@heyitsanthony
Copy link
Contributor Author

@xiang90 excellent. I was planning on slipping an address space ID (regular kv space is asid 0, leases are asid 1, auth is asid 2, etc) into the KV RPCs to do it. Would that make sense?

@xiang90
Copy link
Contributor

xiang90 commented Apr 6, 2016

@heyitsanthony

I am curious about:

  1. how will this change existing KV api.
  2. how do we make sure the leases and kv space are consistent? For key space we have revisions, but for lease and auth we do not. I feel we have to return all leases, auths atomically with the current revision. And then scan the kv space with that revision?

@heyitsanthony
Copy link
Contributor Author

@xiang90

  1. There will be an extra asid int64 in the Get, Put, Delete, and Watch RPC requests. If the asid is 0, it will behave as it does now. If non-zero, the request will be forwarded to the KV implementation that matches the asid.
  2. It seems like that can be done-- resp= Txn().Then(Get(*, asid=leases), Get(*,asid=auth)) and snapshot the kv space at resp.hdr.Revision.

@xiang90
Copy link
Contributor

xiang90 commented Apr 6, 2016

@heyitsanthony So user will be able to get a lease key/value out of our KV API if they specify asid = 1? I feel we should just disable this. And only expose this to the internal snapshot usage.

One more thing is that we want to do all these to make API simpler and enable incremental snapshot (I assume auth/lease are much smaller compared to KV space and returning them in one request is not a big deal). Or we could simply add a Snapshot API into Maintenance Service?

@heyitsanthony
Copy link
Contributor Author

@xiang90
Data in asid > 0 can be meant for internal use only. Would it be hidden by only being accessible through raw KV grpc calls?

I'm hesitant to add a binary blob snapshot API because the KV API seems descriptive enough to take snapshots. On the other hand, the KV approach could end up being super complicated and much more unreliable than sending boltdb copies around. I guess I'll try the snapshot api db blob thing...

@xiang90
Copy link
Contributor

xiang90 commented Apr 7, 2016

Data in asid > 0 can be meant for internal use only. Would it be hidden by only being accessible through raw KV grpc calls?

Yea... This is what I would expect it to work.

I guess I'll try the snapshot api db blob thing...

Then if user wants to do incremental snapshot, we probably need to feed them with raw wal files. But it is doable and simpler from implementation perspective.

@heyitsanthony
Copy link
Contributor Author

closing in favor of #5012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants