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

[Feature] delete event should optionally provide previous value #4620

Closed
hongchaodeng opened this issue Feb 25, 2016 · 12 comments · Fixed by #5880
Closed

[Feature] delete event should optionally provide previous value #4620

hongchaodeng opened this issue Feb 25, 2016 · 12 comments · Fixed by #5880
Milestone

Comments

@hongchaodeng
Copy link
Contributor

Currently in v3 watch workflow, delete event doesn't contain the previous value of deleted key.

If you want to know the previous value, you have to do another Get(key, rev-1) call. This is really bad because it's not simple at all.

There are some scenarios I can think of that wants to make use of previous value. For example, the value might contain indexes different than the key, and it needs to clear those indexes if deleted; Users might watch from 0 and want to know (e.g. logging) the old value if deleted.

None of the above scenarios can be achieved easily without getting the value from delete event.

I'm suggesting that we should add to watch an option that enables attaching the value in event. In this way, we can keep simplicity of the API and gain ease of use.

@xiang90 xiang90 added this to the v3.0.0-maybe milestone Feb 25, 2016
@hongchaodeng
Copy link
Contributor Author

Once we have done client caching, this can be done completely on client side.

@smarterclayton
Copy link
Contributor

The watch filters for labels and for fields depends on this in Kube to know whether to send the last event for a deleted item to the client (nodes for pods). We may also depend on it for graceful deletion in a few cases. The central etcd watch cache could handle this if necessary as noted, but I think returning the deleted item as the previous value was extremely useful for clients in v2.

@xiang90
Copy link
Contributor

xiang90 commented Mar 9, 2016

@smarterclayton You can do it today with a simple Txn that does a get then a delete. We are considering to add a shortcut for this directly into delete though.

@nekto0n
Copy link

nekto0n commented Apr 26, 2016

But what if there are several independent clients, one deletes item, another one - watches. In that case Txn will not help.

@xiang90
Copy link
Contributor

xiang90 commented Apr 26, 2016

@nekto0n For the watch case, the client needs to issue an additional get request to read old version of keys. This is what we try to optimize for, to reduce one round trip time. But it is still doable today.

@purpleidea
Copy link
Contributor

@nekto0n I have the same issue. I think @xiang90 is right that's the only way. If the caching that @hongchaodeng talks about gets added this would be easier. If you find a different solution, please let me know. Thanks!

@xiang90
Copy link
Contributor

xiang90 commented May 31, 2016

@purpleidea We are tying to get this feature in 3.0 if possible. But it is likely that it will not happen in 3.0.

@purpleidea
Copy link
Contributor

@xiang90 that's fine with me. I'm happy with the workaround for now. Keep up the great work :)

@xiang90 xiang90 modified the milestones: v3.1.0, v3.0.0-maybe Jun 17, 2016
@xiang90
Copy link
Contributor

xiang90 commented Jun 21, 2016

@ajityagaty Do you want to take a look at this one?

@ajityagaty
Copy link
Contributor

@xiang90 Sure ! I'll give it a try. Thanks !

@ajityagaty
Copy link
Contributor

@xiang90 Sorry about this one. I couldn't spend time on it.

@xiang90
Copy link
Contributor

xiang90 commented Jul 6, 2016

@ajityagaty No problem at all. It would always be great when there are more eyes on the issues.

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

Successfully merging a pull request may close this issue.

6 participants