From b02798e946cb4d2bb94fef3e78dc62f389243d51 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Fri, 13 Oct 2023 13:47:19 +0200 Subject: [PATCH] Refactor and reorder validation to avoid reporting multiple corelated failures It doesn't make sense to report watch failure if key value operations are not linearizable. Signed-off-by: Marek Siarkowicz --- tests/robustness/validate/operations.go | 24 +++---- tests/robustness/validate/validate.go | 77 +++++++++++++++++++++- tests/robustness/validate/validate_test.go | 6 +- tests/robustness/validate/watch.go | 67 +------------------ 4 files changed, 92 insertions(+), 82 deletions(-) diff --git a/tests/robustness/validate/operations.go b/tests/robustness/validate/operations.go index a0df260e98b..c692e54d590 100644 --- a/tests/robustness/validate/operations.go +++ b/tests/robustness/validate/operations.go @@ -28,28 +28,21 @@ import ( "go.etcd.io/etcd/tests/v3/robustness/model" ) -func validateOperationsAndVisualize(t *testing.T, lg *zap.Logger, operations []porcupine.Operation, eventHistory []model.WatchEvent) func(basepath string) error { +func validateLinearizableOperationsAndVisualize(lg *zap.Logger, operations []porcupine.Operation) (result porcupine.CheckResult, visualize func(basepath string) error) { const timeout = 5 * time.Minute lg.Info("Validating linearizable operations", zap.Duration("timeout", timeout)) - result, visualize := validateLinearizableOperationAndVisualize(lg, operations, timeout) + result, info := porcupine.CheckOperationsVerbose(model.NonDeterministicModel, operations, timeout) switch result { case porcupine.Illegal: - t.Error("Linearization failed") + lg.Error("Linearization failed") case porcupine.Unknown: - t.Error("Linearization has timed out") + lg.Error("Linearization has timed out") case porcupine.Ok: - t.Log("Linearization success") - lg.Info("Validating serializable operations") - validateSerializableOperations(t, operations, eventHistory) + lg.Info("Linearization success") default: - t.Fatalf("Unknown Linearization") + panic(fmt.Sprintf("Unknown Linearization result %s", result)) } - return visualize -} - -func validateLinearizableOperationAndVisualize(lg *zap.Logger, operations []porcupine.Operation, timeout time.Duration) (result porcupine.CheckResult, visualize func(basepath string) error) { - linearizable, info := porcupine.CheckOperationsVerbose(model.NonDeterministicModel, operations, timeout) - return linearizable, func(path string) error { + return result, func(path string) error { lg.Info("Saving visualization", zap.String("path", path)) err := porcupine.VisualizePath(model.NonDeterministicModel, info, path) if err != nil { @@ -59,7 +52,8 @@ func validateLinearizableOperationAndVisualize(lg *zap.Logger, operations []porc } } -func validateSerializableOperations(t *testing.T, operations []porcupine.Operation, totalEventHistory []model.WatchEvent) { +func validateSerializableOperations(t *testing.T, lg *zap.Logger, operations []porcupine.Operation, totalEventHistory []model.WatchEvent) { + lg.Info("Validating serializable operations") staleReads := filterSerializableReads(operations) if len(staleReads) == 0 { return diff --git a/tests/robustness/validate/validate.go b/tests/robustness/validate/validate.go index 5614737a47d..401899a9d05 100644 --- a/tests/robustness/validate/validate.go +++ b/tests/robustness/validate/validate.go @@ -15,20 +15,93 @@ package validate import ( + "fmt" + "sort" "testing" + "github.com/anishathalye/porcupine" + "github.com/google/go-cmp/cmp" "go.uber.org/zap" + "go.etcd.io/etcd/tests/v3/robustness/model" "go.etcd.io/etcd/tests/v3/robustness/report" ) // ValidateAndReturnVisualize returns visualize as porcupine.linearizationInfo used to generate visualization is private. func ValidateAndReturnVisualize(t *testing.T, lg *zap.Logger, cfg Config, reports []report.ClientReport) (visualize func(basepath string) error) { - eventHistory := validateWatch(t, cfg, reports) patchedOperations := patchedOperationHistory(reports) - return validateOperationsAndVisualize(t, lg, patchedOperations, eventHistory) + linearizable, visualize := validateLinearizableOperationsAndVisualize(lg, patchedOperations) + if linearizable != porcupine.Ok { + lg.Error("Failed linearization, skipping further validation") + return visualize + } + // TODO: Don't use watch events to get event history. + eventHistory, err := mergeWatchEventHistory(reports) + if err != nil { + t.Errorf("Failed merging watch history to create event history, skipping further validation, err: %s", err) + return visualize + } + validateWatch(t, lg, cfg, reports, eventHistory) + validateSerializableOperations(t, lg, patchedOperations, eventHistory) + return visualize } type Config struct { ExpectRevisionUnique bool } + +func mergeWatchEventHistory(reports []report.ClientReport) ([]model.WatchEvent, error) { + type revisionEvents struct { + events []model.WatchEvent + revision int64 + clientId int + } + revisionToEvents := map[int64]revisionEvents{} + var lastClientId = 0 + var lastRevision int64 + events := []model.WatchEvent{} + for _, r := range reports { + for _, op := range r.Watch { + for _, resp := range op.Responses { + for _, event := range resp.Events { + if event.Revision == lastRevision && lastClientId == r.ClientId { + events = append(events, event) + } else { + if prev, found := revisionToEvents[lastRevision]; found { + // This assumes that there are txn that would be observed differently by two watches. + // TODO: Implement merging events from multiple watches about single revision based on operations. + if diff := cmp.Diff(prev.events, events); diff != "" { + return nil, fmt.Errorf("events between clients %d and %d don't match, revision: %d, diff: %s", prev.clientId, lastClientId, lastRevision, diff) + } + } else { + revisionToEvents[lastRevision] = revisionEvents{clientId: lastClientId, events: events, revision: lastRevision} + } + lastClientId = r.ClientId + lastRevision = event.Revision + events = []model.WatchEvent{event} + } + } + } + } + } + if prev, found := revisionToEvents[lastRevision]; found { + if diff := cmp.Diff(prev.events, events); diff != "" { + return nil, fmt.Errorf("events between clients %d and %d don't match, revision: %d, diff: %s", prev.clientId, lastClientId, lastRevision, diff) + } + } else { + revisionToEvents[lastRevision] = revisionEvents{clientId: lastClientId, events: events, revision: lastRevision} + } + + var allRevisionEvents []revisionEvents + for _, revEvents := range revisionToEvents { + allRevisionEvents = append(allRevisionEvents, revEvents) + } + sort.Slice(allRevisionEvents, func(i, j int) bool { + return allRevisionEvents[i].revision < allRevisionEvents[j].revision + }) + var eventHistory []model.WatchEvent + for _, revEvents := range allRevisionEvents { + eventHistory = append(eventHistory, revEvents.events...) + } + return eventHistory, nil +} diff --git a/tests/robustness/validate/validate_test.go b/tests/robustness/validate/validate_test.go index 7a50ef8621e..517f8a9657d 100644 --- a/tests/robustness/validate/validate_test.go +++ b/tests/robustness/validate/validate_test.go @@ -211,7 +211,11 @@ func TestValidateWatch(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - validateWatch(t, Config{ExpectRevisionUnique: true}, tc.reports) + eventHistory, err := mergeWatchEventHistory(tc.reports) + if err != nil { + t.Fatal(err) + } + validateWatch(t, zaptest.NewLogger(t), Config{ExpectRevisionUnique: true}, tc.reports, eventHistory) }) } } diff --git a/tests/robustness/validate/watch.go b/tests/robustness/validate/watch.go index e71e7bc49a1..a17130e9066 100644 --- a/tests/robustness/validate/watch.go +++ b/tests/robustness/validate/watch.go @@ -15,27 +15,22 @@ package validate import ( - "sort" "testing" - "github.com/google/go-cmp/cmp" + "go.uber.org/zap" "go.etcd.io/etcd/tests/v3/robustness/model" "go.etcd.io/etcd/tests/v3/robustness/report" ) -func validateWatch(t *testing.T, cfg Config, reports []report.ClientReport) []model.WatchEvent { +func validateWatch(t *testing.T, lg *zap.Logger, cfg Config, reports []report.ClientReport, eventHistory []model.WatchEvent) []model.WatchEvent { + lg.Info("Validating watch") // Validate etcd watch properties defined in https://etcd.io/docs/v3.6/learning/api_guarantees/#watch-apis for _, r := range reports { validateOrdered(t, r) validateUnique(t, cfg.ExpectRevisionUnique, r) validateAtomic(t, r) validateBookmarkable(t, r) - } - // TODO: Use linearization result instead of event history to get order of events - // This is currently impossible as porcupine doesn't expose operation order created during linearization. - eventHistory := mergeWatchEventHistory(t, reports) - for _, r := range reports { validateReliable(t, eventHistory, r) validateResumable(t, eventHistory, r) } @@ -165,59 +160,3 @@ func firstWatchEvent(op model.WatchOperation) *model.WatchEvent { } return nil } - -func mergeWatchEventHistory(t *testing.T, reports []report.ClientReport) []model.WatchEvent { - type revisionEvents struct { - events []model.WatchEvent - revision int64 - clientId int - } - revisionToEvents := map[int64]revisionEvents{} - var lastClientId = 0 - var lastRevision int64 - events := []model.WatchEvent{} - for _, r := range reports { - for _, op := range r.Watch { - for _, resp := range op.Responses { - for _, event := range resp.Events { - if event.Revision == lastRevision && lastClientId == r.ClientId { - events = append(events, event) - } else { - if prev, found := revisionToEvents[lastRevision]; found { - // This assumes that there are txn that would be observed differently by two watches. - // TODO: Implement merging events from multiple watches about single revision based on operations. - if diff := cmp.Diff(prev.events, events); diff != "" { - t.Errorf("Events between clients %d and %d don't match, revision: %d, diff: %s", prev.clientId, lastClientId, lastRevision, diff) - } - } else { - revisionToEvents[lastRevision] = revisionEvents{clientId: lastClientId, events: events, revision: lastRevision} - } - lastClientId = r.ClientId - lastRevision = event.Revision - events = []model.WatchEvent{event} - } - } - } - } - } - if prev, found := revisionToEvents[lastRevision]; found { - if diff := cmp.Diff(prev.events, events); diff != "" { - t.Errorf("Events between clients %d and %d don't match, revision: %d, diff: %s", prev.clientId, lastClientId, lastRevision, diff) - } - } else { - revisionToEvents[lastRevision] = revisionEvents{clientId: lastClientId, events: events, revision: lastRevision} - } - - var allRevisionEvents []revisionEvents - for _, revEvents := range revisionToEvents { - allRevisionEvents = append(allRevisionEvents, revEvents) - } - sort.Slice(allRevisionEvents, func(i, j int) bool { - return allRevisionEvents[i].revision < allRevisionEvents[j].revision - }) - var eventHistory []model.WatchEvent - for _, revEvents := range allRevisionEvents { - eventHistory = append(eventHistory, revEvents.events...) - } - return eventHistory -}