cockroachdb / cockroachdb/cockroach
sidetransport: rethink whether closed ts updates should be skipped for Subsume or Lease only requests
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
Currently, the side transport checks with leaseholder replicas before sending closed timestamp updates. If there are pending raft proposals, leaseholder replicas would choose to skip publishing the closed timestamp updates, assuming those proposals will carry the updates already. https://github.com/cockroachdb/cockroach/blob/5dd4f88d08a4de528b96857cd611b42342b26a65/pkg/kv/kvserver/replica_closedts.go#L124
However, this assumption may fail - requests like SubsumeRequest and LeaseRequest do not include closed timestamp updates. https://github.com/cockroachdb/cockroach/blob/2d677b744b1e836ef29b4691e875f8d062d8d2e4/pkg/kv/kvserver/replica_proposal_buf.go#L717-L770
A proposed fix is to check when only Lease or Subsume requests are pending and allow side transport to publish closed timestamp updates in those cases. However, more thoughts need to be put to understand the implications of such change.
The consequence of this behavior is that global reads may fail to serve on follower replicas. We assume that closed timestamp updates will arrive at followers by:
```
closed_ts_at_sender = now(sender’s current clock) + maxClockOffset(500ms by
default) + buffer (25ms) + max(raft_propagation_time,side_propagation_time)
```
However, if closed timestamp updates are skipped (e.g., due to pending Lease or Subsume requests) from side transport, and raft updates also do not carry closed timestamp updates, `max(raft_propagation_time, side_propagation_time)` is no longer sufficient to guarantee timely delivery of closed timestamp updates. In such cases, follower reads may need to be redirected to the leaseholder.
Contributor guide
Research direction
Start with the linked sections in pkg/kv/kvserver/replica_closedts.go and pkg/kv/kvserver/replica_proposal_buf.go, focusing on side-transport checks and SubsumeRequest or LeaseRequest proposals. Trace how closed timestamp updates reach follower replicas and assess the proposed exception. Done means the behavior and its implications for follower reads are resolved, with validation added where the repository supports it.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 28/100