Skip to content

Commit

Permalink
Refactor and reorder validation to avoid reporting multiple corelated…
Browse files Browse the repository at this point in the history 
… failures

It doesn't make sense to report watch failure if key value operations
are not linearizable.

Signed-off-by: Marek Siarkowicz <[email protected]>
  • Loading branch information
@serathius
serathius committed Oct 13, 2023
1 parent f198b41 commit b02798e
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 82 deletions.
24 changes: 9 additions & 15 deletions tests/robustness/validate/operations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
77 changes: 75 additions & 2 deletions tests/robustness/validate/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
6 changes: 5 additions & 1 deletion tests/robustness/validate/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
67 changes: 3 additions & 64 deletions tests/robustness/validate/watch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

0 comments on commit b02798e

Please sign in to comment.