Merge pull request #16695 from serathius/watch-validation-revision

Fix watch validation assuming that client requesting older watch revision
etcd-io · Oct 5, 2023 · 3f859a6 · 3f859a6
2 parents 6a96ab7 + c2655b4
commit 3f859a6
Show file tree

Hide file tree

Showing 2 changed files with 172 additions and 5 deletions.
diff --git a/tests/robustness/validate/validate_test.go b/tests/robustness/validate/validate_test.go
@@ -23,6 +23,7 @@ import (
 	"go.uber.org/zap/zaptest"
 
 	"go.etcd.io/etcd/tests/v3/framework/testutils"
+	"go.etcd.io/etcd/tests/v3/robustness/model"
 	"go.etcd.io/etcd/tests/v3/robustness/report"
 )
 
@@ -47,3 +48,170 @@ func TestValidate(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateWatch(t *testing.T) {
+	tcs := []struct {
+		name    string
+		reports []report.ClientReport
+	}{
+		{
+			name: "earlier event after bookmark in separate request",
+			reports: []report.ClientReport{
+				{
+					Watch: []model.WatchOperation{
+						{
+							Request: model.WatchRequest{
+								Key:      "a",
+								Revision: 100,
+							},
+							Responses: []model.WatchResponse{
+								{
+									IsProgressNotify: true,
+									Revision:         100,
+								},
+							},
+						},
+						{
+							Request: model.WatchRequest{
+								Key:      "a",
+								Revision: 99,
+							},
+							Responses: []model.WatchResponse{
+								{
+									Events: []model.WatchEvent{
+										{
+											Event: model.Event{
+												Type: model.PutOperation,
+												Key:  "a",
+												Value: model.ValueOrHash{
+													Value: "99",
+												},
+											},
+											Revision: 99,
+										},
+									},
+									Revision: 100,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "earlier event after in separate request",
+			reports: []report.ClientReport{
+				{
+					Watch: []model.WatchOperation{
+						{
+							Request: model.WatchRequest{
+								Key:      "a",
+								Revision: 100,
+							},
+							Responses: []model.WatchResponse{
+								{
+									Events: []model.WatchEvent{
+										{
+											Event: model.Event{
+												Type: model.PutOperation,
+												Key:  "a",
+												Value: model.ValueOrHash{
+													Value: "100",
+												},
+											},
+											Revision: 100,
+										},
+									},
+									Revision: 100,
+								},
+							},
+						},
+						{
+							Request: model.WatchRequest{
+								Key:      "a",
+								Revision: 99,
+							},
+							Responses: []model.WatchResponse{
+								{
+									Events: []model.WatchEvent{
+										{
+											Event: model.Event{
+												Type: model.PutOperation,
+												Key:  "a",
+												Value: model.ValueOrHash{
+													Value: "99",
+												},
+											},
+											Revision: 99,
+										},
+									},
+									Revision: 100,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "duplicated event between two separate requests",
+			reports: []report.ClientReport{
+				{
+					Watch: []model.WatchOperation{
+						{
+							Request: model.WatchRequest{
+								Key:      "a",
+								Revision: 100,
+							},
+							Responses: []model.WatchResponse{
+								{
+									Events: []model.WatchEvent{
+										{
+											Event: model.Event{
+												Type: model.PutOperation,
+												Key:  "a",
+												Value: model.ValueOrHash{
+													Value: "100",
+												},
+											},
+											Revision: 100,
+										},
+									},
+									Revision: 100,
+								},
+							},
+						},
+						{
+							Request: model.WatchRequest{
+								Key:      "a",
+								Revision: 100,
+							},
+							Responses: []model.WatchResponse{
+								{
+									Events: []model.WatchEvent{
+										{
+											Event: model.Event{
+												Type: model.PutOperation,
+												Key:  "a",
+												Value: model.ValueOrHash{
+													Value: "100",
+												},
+											},
+											Revision: 100,
+										},
+									},
+									Revision: 100,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			validateWatch(t, Config{ExpectRevisionUnique: true}, tc.reports)
+		})
+	}
+}
diff --git a/tests/robustness/validate/watch.go b/tests/robustness/validate/watch.go
@@ -43,8 +43,8 @@ func validateWatch(t *testing.T, cfg Config, reports []report.ClientReport) []mo
 }
 
 func validateBookmarkable(t *testing.T, report report.ClientReport) {
-	var lastProgressNotifyRevision int64
 	for _, op := range report.Watch {
+		var lastProgressNotifyRevision int64
 		for _, resp := range op.Responses {
 			for _, event := range resp.Events {
 				if event.Revision <= lastProgressNotifyRevision {
@@ -59,8 +59,8 @@ func validateBookmarkable(t *testing.T, report report.ClientReport) {
 }
 
 func validateOrdered(t *testing.T, report report.ClientReport) {
-	var lastEventRevision int64 = 1
 	for _, op := range report.Watch {
+		var lastEventRevision int64 = 1
 		for _, resp := range op.Responses {
 			for _, event := range resp.Events {
 				if event.Revision < lastEventRevision {
@@ -73,9 +73,8 @@ func validateOrdered(t *testing.T, report report.ClientReport) {
 }
 
 func validateUnique(t *testing.T, expectUniqueRevision bool, report report.ClientReport) {
-	uniqueOperations := map[any]struct{}{}
-
 	for _, op := range report.Watch {
+		uniqueOperations := map[any]struct{}{}
 		for _, resp := range op.Responses {
 			for _, event := range resp.Events {
 				var key any
@@ -97,8 +96,8 @@ func validateUnique(t *testing.T, expectUniqueRevision bool, report report.Clien
 }
 
 func validateAtomic(t *testing.T, report report.ClientReport) {
-	var lastEventRevision int64 = 1
 	for _, op := range report.Watch {
+		var lastEventRevision int64 = 1
 		for _, resp := range op.Responses {
 			if len(resp.Events) > 0 {
 				if resp.Events[0].Revision == lastEventRevision {