prevKV not being returned if the previous KV was compacted is suprising behavior

[jpbetz]

See https://github.com/kubernetes/kubernetes/issues/76624

For delete watch event, in particular, I'm guessing the conditions for this to happen would be something like:

1: put "/x" -> "value" (revision=1) 2. create watch "/x" 3. delete "/x" (revision=2) 4. compaction 5. "/x deleted" watch event sent

Because if the compaction happened before (3) or after (5), the prevKV would be included in the "/x deleted" watch event (revision 1 of "/x" can't be compacted until after the delete). I'm concerned this is a rare enough situation that clients that make use of prevKV are unlikely to see it in development and even more unlikely to write code to defend against it until it breaks in production.

One option would be to terminate the watch with ErrCompacted rather than returning a watch event with a missing prevKV. Clients would need to re-establish the watch if they get the ErrCompacted error, but they need to be written to be able to do that already. Only watches with WithPrevKV() enabled would be impacted.

Another would be to modify compaction to retain both the latest and the previous of each KV. This would 2x minimum disk space in the worst case.

[jpbetz]

cc @liggitt @jingyih @xiang90

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

[liggitt]

Do watches ever end with a compacted error today?

[xiang90]

@liggitt

It should return errCompacted.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

[serathius]

Would like to revisit this as part of Kubernetes-etcd contract.