Skip to content

Commit 5ec40e3

Browse files
authored
Merge pull request #20614 from apullo777/progress-notify
cache: make the cache progress-aware
2 parents d67e847 + ed1d377 commit 5ec40e3

File tree

7 files changed

+224
-38
lines changed

7 files changed

+224
-38
lines changed

cache/cache.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -271,13 +271,7 @@ func (c *Cache) applyStorage(storeW *watcher) error {
271271
if !ok {
272272
return nil
273273
}
274-
if resp.Canceled {
275-
return nil
276-
}
277-
if len(resp.Events) == 0 {
278-
continue
279-
}
280-
if err := c.store.Apply(resp.Events); err != nil {
274+
if err := c.store.Apply(resp); err != nil {
281275
return err
282276
}
283277
}

cache/demux.go

Lines changed: 72 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@ package cache
1616

1717
import (
1818
"context"
19+
"errors"
1920
"sync"
2021
"time"
2122

23+
"go.etcd.io/etcd/api/v3/etcdserverpb"
2224
clientv3 "go.etcd.io/etcd/client/v3"
2325
)
2426

@@ -108,6 +110,9 @@ func (d *demux) Unregister(w *watcher) {
108110
func (d *demux) Init(minRev int64) {
109111
d.mu.Lock()
110112
defer d.mu.Unlock()
113+
if minRev == 0 {
114+
return
115+
}
111116
if d.minRev == 0 {
112117
// Watch started for empty demux
113118
d.minRev = minRev
@@ -129,23 +134,35 @@ func (d *demux) Init(minRev int64) {
129134
}
130135

131136
func (d *demux) Broadcast(resp clientv3.WatchResponse) error {
132-
events := resp.Events
133-
if len(events) == 0 {
134-
return nil
135-
}
136-
137137
d.mu.Lock()
138138
defer d.mu.Unlock()
139-
err := validateRevisions(events, d.maxRev)
139+
if d.minRev == 0 {
140+
return errors.New("demux: not initialized")
141+
}
142+
err := validateRevisions(resp, d.maxRev)
140143
if err != nil {
141144
return err
142145
}
143-
d.updateStoreLocked(events)
144-
d.broadcastLocked(events)
146+
d.updateStoreLocked(resp)
147+
d.broadcastLocked(resp)
145148
return nil
146149
}
147150

148-
func (d *demux) updateStoreLocked(events []*clientv3.Event) {
151+
func (d *demux) LatestRev() int64 {
152+
d.mu.RLock()
153+
defer d.mu.RUnlock()
154+
return d.maxRev
155+
}
156+
157+
func (d *demux) updateStoreLocked(resp clientv3.WatchResponse) {
158+
if resp.IsProgressNotify() {
159+
d.maxRev = resp.Header.Revision
160+
return
161+
}
162+
if len(resp.Events) == 0 {
163+
return
164+
}
165+
events := resp.Events
149166
batchStart := 0
150167
for end := 1; end < len(events); end++ {
151168
if events[end].Kv.ModRevision != events[batchStart].Kv.ModRevision {
@@ -167,7 +184,33 @@ func (d *demux) updateStoreLocked(events []*clientv3.Event) {
167184
d.maxRev = events[len(events)-1].Kv.ModRevision
168185
}
169186

170-
func (d *demux) broadcastLocked(events []*clientv3.Event) {
187+
func (d *demux) broadcastLocked(resp clientv3.WatchResponse) {
188+
switch {
189+
case resp.IsProgressNotify():
190+
d.broadcastProgressLocked(resp.Header.Revision)
191+
case len(resp.Events) != 0:
192+
d.broadcastEventsLocked(resp.Events)
193+
default:
194+
}
195+
}
196+
197+
func (d *demux) broadcastProgressLocked(progressRev int64) {
198+
for w, nextRev := range d.activeWatchers {
199+
if nextRev >= progressRev {
200+
continue
201+
}
202+
resp := clientv3.WatchResponse{
203+
Header: etcdserverpb.ResponseHeader{
204+
Revision: progressRev,
205+
},
206+
}
207+
if w.enqueueResponse(resp) {
208+
d.activeWatchers[w] = progressRev + 1
209+
}
210+
}
211+
}
212+
213+
func (d *demux) broadcastEventsLocked(events []*clientv3.Event) {
171214
firstRev := events[0].Kv.ModRevision
172215
lastRev := events[len(events)-1].Kv.ModRevision
173216

@@ -177,18 +220,22 @@ func (d *demux) broadcastLocked(events []*clientv3.Event) {
177220
delete(d.activeWatchers, w)
178221
continue
179222
}
223+
180224
sendStart := len(events)
181225
for i, ev := range events {
182226
if ev.Kv.ModRevision >= nextRev {
183227
sendStart = i
184228
break
185229
}
186230
}
231+
187232
if sendStart == len(events) {
188233
continue
189234
}
190235

191-
if !w.enqueueEvent(events[sendStart:]) { // overflow → lagging
236+
if !w.enqueueResponse(clientv3.WatchResponse{
237+
Events: events[sendStart:],
238+
}) { // overflow → lagging
192239
d.laggingWatchers[w] = nextRev
193240
delete(d.activeWatchers, w)
194241
} else {
@@ -241,17 +288,26 @@ func (d *demux) resyncLaggingWatchers() {
241288
continue
242289
}
243290
// TODO: re-enable key‐predicate in Filter when non‐zero startRev or performance tuning is needed
244-
enqueueFailed := false
291+
resyncSuccess := true
245292
d.history.AscendGreaterOrEqual(nextRev, func(rev int64, eventBatch []*clientv3.Event) bool {
246-
if !w.enqueueEvent(eventBatch) { // buffer overflow: watcher still lagging
247-
enqueueFailed = true
293+
resp := clientv3.WatchResponse{
294+
Events: eventBatch,
295+
}
296+
if !w.enqueueResponse(resp) { // buffer overflow: watcher still lagging
297+
resyncSuccess = false
248298
return false
249299
}
250300
nextRev = rev + 1
251301
return true
252302
})
253-
254-
if !enqueueFailed {
303+
// Send progress to just resync.
304+
if resyncSuccess {
305+
resp := clientv3.WatchResponse{
306+
Header: etcdserverpb.ResponseHeader{Revision: d.maxRev},
307+
}
308+
if d.maxRev > nextRev && w.enqueueResponse(resp) {
309+
nextRev = d.maxRev + 1
310+
}
255311
delete(d.laggingWatchers, w)
256312
d.activeWatchers[w] = nextRev
257313
} else {

cache/demux_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ func TestBroadcastBatching(t *testing.T) {
257257
t.Run(tt.name, func(t *testing.T) {
258258
d := newDemux(16, 10*time.Millisecond)
259259
w := newWatcher(len(tt.input)+1, nil)
260+
d.Init(1)
260261
d.Register(w, 0)
261262

262263
d.Broadcast(respWithEventRevs(tt.input...))
@@ -305,6 +306,7 @@ func TestSlowWatcherResync(t *testing.T) {
305306
t.Run(tt.name, func(t *testing.T) {
306307
d := newDemux(16, 10*time.Millisecond)
307308
w := newWatcher(1, nil)
309+
d.Init(1)
308310
d.Register(w, 0)
309311

310312
d.Broadcast(respWithEventRevs(tt.input...))

cache/store.go

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package cache
1616

1717
import (
18+
"errors"
1819
"fmt"
1920
"sync"
2021

@@ -92,6 +93,10 @@ func (s *store) getSnapshot(rev int64) (*snapshot, int64, error) {
9293
targetSnapshot = snap
9394
return false
9495
})
96+
// If s.history < rev < s.latest.rev serve latest.
97+
if targetSnapshot == nil {
98+
targetSnapshot = &s.latest
99+
}
95100

96101
return targetSnapshot, s.latest.rev, nil
97102
}
@@ -110,14 +115,36 @@ func (s *store) Restore(kvs []*mvccpb.KeyValue, rev int64) {
110115
s.history.Append(newClonedSnapshot(rev, s.latest.tree))
111116
}
112117

113-
func (s *store) Apply(events []*clientv3.Event) error {
118+
func (s *store) Apply(resp clientv3.WatchResponse) error {
119+
if resp.Canceled {
120+
return errors.New("canceled")
121+
}
122+
114123
s.mu.Lock()
115124
defer s.mu.Unlock()
116-
117-
if err := validateRevisions(events, s.latest.rev); err != nil {
125+
if err := validateRevisions(resp, s.latest.rev); err != nil {
118126
return err
119127
}
120128

129+
switch {
130+
case resp.IsProgressNotify():
131+
s.applyProgressNotifyLocked(resp.Header.Revision)
132+
return nil
133+
case len(resp.Events) != 0:
134+
return s.applyEventsLocked(resp.Events)
135+
default:
136+
return nil
137+
}
138+
}
139+
140+
func (s *store) applyProgressNotifyLocked(revision int64) {
141+
if s.latest.rev == 0 {
142+
return
143+
}
144+
s.latest.rev = revision
145+
}
146+
147+
func (s *store) applyEventsLocked(events []*clientv3.Event) error {
121148
for i := 0; i < len(events); {
122149
rev := events[i].Kv.ModRevision
123150

@@ -145,7 +172,14 @@ func (s *store) LatestRev() int64 {
145172
return s.latest.rev
146173
}
147174

148-
func validateRevisions(events []*clientv3.Event, latestRev int64) error {
175+
func validateRevisions(resp clientv3.WatchResponse, latestRev int64) error {
176+
if resp.IsProgressNotify() {
177+
if resp.Header.Revision < latestRev {
178+
return fmt.Errorf("cache: progress notification out of order (progress %d < latest %d)", resp.Header.Revision, latestRev)
179+
}
180+
return nil
181+
}
182+
events := resp.Events
149183
if len(events) == 0 {
150184
return nil
151185
}

cache/store_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,8 @@ func TestStoreApply(t *testing.T) {
281281

282282
var gotErr error
283283
for batchIndex, batch := range test.eventBatches {
284-
if err := s.Apply(batch); err != nil {
284+
resp := clientv3.WatchResponse{Events: batch}
285+
if err := s.Apply(resp); err != nil {
285286
gotErr = err
286287
if !test.expectErr {
287288
t.Fatalf("Apply(batch %d) unexpected error: %v", batchIndex, err)
@@ -397,7 +398,8 @@ func TestRestoreAppendCloneImmutability(t *testing.T) {
397398
s.Restore(test.initialKVs, test.initialRev)
398399
}
399400
if len(test.events) > 0 {
400-
if err := s.Apply(test.events); err != nil {
401+
resp := clientv3.WatchResponse{Events: test.events}
402+
if err := s.Apply(resp); err != nil {
401403
t.Fatalf("Apply failed: %v", err)
402404
}
403405
}

cache/watcher.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,21 @@ func newWatcher(bufSize int, pred KeyPredicate) *watcher {
3838

3939
// true -> events delivered (or filtered/duplicate)
4040
// false -> buffer full (caller should mark watcher “lagging”)
41-
func (w *watcher) enqueueEvent(eventBatch []*clientv3.Event) bool {
42-
if w.keyPred != nil {
43-
filtered := make([]*clientv3.Event, 0, len(eventBatch))
44-
for _, event := range eventBatch {
41+
func (w *watcher) enqueueResponse(resp clientv3.WatchResponse) bool {
42+
if !resp.IsProgressNotify() && w.keyPred != nil {
43+
filtered := make([]*clientv3.Event, 0, len(resp.Events))
44+
for _, event := range resp.Events {
4545
if w.keyPred(event.Kv.Key) {
4646
filtered = append(filtered, event)
4747
}
4848
}
4949
if len(filtered) == 0 {
5050
return true
5151
}
52-
eventBatch = filtered
52+
resp.Events = filtered
5353
}
5454
select {
55-
case w.respCh <- clientv3.WatchResponse{Events: eventBatch}:
55+
case w.respCh <- resp:
5656
return true
5757
default:
5858
return false

0 commit comments

Comments
 (0)