Validate bookmarkable checks the last event before progress notify

Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
This commit is contained in:
Marek Siarkowicz 2024-04-10 20:52:54 +02:00
parent 6f6647271f
commit dc187ce6e8
2 changed files with 31 additions and 15 deletions

View File

@ -493,8 +493,7 @@ func TestValidateWatch(t *testing.T) {
putPersistedEvent("b", "2", 3, true), putPersistedEvent("b", "2", 3, true),
putPersistedEvent("c", "3", 4, true), putPersistedEvent("c", "3", 4, true),
}, },
// TODO: Should fail as it should guarantee that all events between requested revision and bookmark are delivered expectError: errBrokeBookmarkable.Error(),
expectError: "",
}, },
{ {
name: "Bookmarkable - revision non decreasing - pass", name: "Bookmarkable - revision non decreasing - pass",
@ -583,8 +582,7 @@ func TestValidateWatch(t *testing.T) {
putPersistedEvent("b", "2", 3, true), putPersistedEvent("b", "2", 3, true),
putPersistedEvent("c", "3", 4, true), putPersistedEvent("c", "3", 4, true),
}, },
// TODO: This should fail as bookmark lowered revision expectError: errBrokeBookmarkable.Error(),
expectError: "",
}, },
{ {
name: "Bookmarkable - progress precedes event - fail", name: "Bookmarkable - progress precedes event - fail",
@ -653,8 +651,7 @@ func TestValidateWatch(t *testing.T) {
putPersistedEvent("b", "2", 3, true), putPersistedEvent("b", "2", 3, true),
putPersistedEvent("c", "3", 4, true), putPersistedEvent("c", "3", 4, true),
}, },
// TODO: This should fail as there is a missing event before bookmark expectError: errBrokeBookmarkable.Error(),
expectError: "",
}, },
{ {
name: "Bookmarkable - missing event matching watch before bookmark - fail", name: "Bookmarkable - missing event matching watch before bookmark - fail",
@ -685,8 +682,7 @@ func TestValidateWatch(t *testing.T) {
putPersistedEvent("a", "2", 3, false), putPersistedEvent("a", "2", 3, false),
putPersistedEvent("c", "3", 4, true), putPersistedEvent("c", "3", 4, true),
}, },
// TODO: This should fail as there is a missing event before bookmark expectError: errBrokeBookmarkable.Error(),
expectError: "",
}, },
{ {
name: "Bookmarkable - missing event matching watch with prefix before bookmark - fail", name: "Bookmarkable - missing event matching watch with prefix before bookmark - fail",
@ -718,8 +714,7 @@ func TestValidateWatch(t *testing.T) {
putPersistedEvent("ab", "2", 3, true), putPersistedEvent("ab", "2", 3, true),
putPersistedEvent("cc", "3", 4, true), putPersistedEvent("cc", "3", 4, true),
}, },
// TODO: This should fail as there is a missing event before bookmark expectError: errBrokeBookmarkable.Error(),
expectError: "",
}, },
{ {
name: "Reliable - all events history - pass", name: "Reliable - all events history - pass",

View File

@ -17,6 +17,7 @@ package validate
import ( import (
"errors" "errors"
"github.com/google/go-cmp/cmp"
"go.uber.org/zap" "go.uber.org/zap"
"go.etcd.io/etcd/tests/v3/robustness/model" "go.etcd.io/etcd/tests/v3/robustness/model"
@ -55,11 +56,11 @@ func validateWatch(lg *zap.Logger, cfg Config, reports []report.ClientReport, ev
if err != nil { if err != nil {
return err return err
} }
err = validateBookmarkable(lg, r)
if err != nil {
return err
}
if eventHistory != nil { if eventHistory != nil {
err = validateBookmarkable(lg, eventHistory, r)
if err != nil {
return err
}
err = validateReliable(lg, eventHistory, r) err = validateReliable(lg, eventHistory, r)
if err != nil { if err != nil {
return err return err
@ -95,17 +96,37 @@ func validateFilter(lg *zap.Logger, report report.ClientReport) (err error) {
return err return err
} }
func validateBookmarkable(lg *zap.Logger, report report.ClientReport) (err error) { func validateBookmarkable(lg *zap.Logger, eventHistory []model.PersistedEvent, report report.ClientReport) (err error) {
for _, op := range report.Watch { for _, op := range report.Watch {
var lastProgressNotifyRevision int64 var lastProgressNotifyRevision int64
var gotEventBeforeProgressNotify *model.PersistedEvent
for _, resp := range op.Responses { for _, resp := range op.Responses {
for _, event := range resp.Events { for _, event := range resp.Events {
if event.Revision <= lastProgressNotifyRevision { if event.Revision <= lastProgressNotifyRevision {
lg.Error("Broke watch guarantee", zap.String("guarantee", "bookmarkable"), zap.Int("client", report.ClientID), zap.Int64("revision", event.Revision)) lg.Error("Broke watch guarantee", zap.String("guarantee", "bookmarkable"), zap.Int("client", report.ClientID), zap.Int64("revision", event.Revision))
err = errBrokeBookmarkable err = errBrokeBookmarkable
} }
gotEventBeforeProgressNotify = &event.PersistedEvent
} }
if resp.IsProgressNotify { if resp.IsProgressNotify {
if gotEventBeforeProgressNotify != nil || op.Request.Revision != 0 {
var wantEventBeforeProgressNotify *model.PersistedEvent
for _, ev := range eventHistory {
if ev.Revision < op.Request.Revision {
continue
}
if ev.Revision > resp.Revision {
break
}
if ev.Match(op.Request) {
wantEventBeforeProgressNotify = &ev
}
}
if diff := cmp.Diff(wantEventBeforeProgressNotify, gotEventBeforeProgressNotify); diff != "" {
lg.Error("Broke watch guarantee", zap.String("guarantee", "bookmarkable"), zap.Int("client", report.ClientID), zap.String("diff", diff))
err = errBrokeBookmarkable
}
}
lastProgressNotifyRevision = resp.Revision lastProgressNotifyRevision = resp.Revision
} }
} }