prevKV not being returned if the previous KV was compacted is suprising behavior
- Dominant language
- Go
- Stars
- 52.3k
- Forks
- 10.5k
- Avg merge
- 2d 21h
- Merged PRs (30d)
- 43
Description
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.
Contributor guide
Research direction
Start by tracing delete watch events with WithPrevKV enabled through compaction, focusing on the ErrCompacted behavior described in the issue's reproduction sequence. No files or tests are named; first determine whether the intended resolution is to terminate the watch or retain the previous KV, then add coverage showing the chosen behavior after compaction.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100