diff --git a/tests/robustness/model/history.go b/tests/robustness/model/history.go index fc224a8afffe..207c933f4672 100644 --- a/tests/robustness/model/history.go +++ b/tests/robustness/model/history.go @@ -290,8 +290,6 @@ func (h *AppendableHistory) appendFailed(request EtcdRequest, start, end time.Du } isRead := request.IsRead() if !isRead { - // Failed writes can still be persisted, setting -1 for now as don't know when request has took effect. - op.Return = -1 // Operations of single client needs to be sequential. // As we don't know return time of failed operations, all new writes need to be done with new stream id. h.streamID = h.idProvider.NewStreamID() @@ -300,7 +298,7 @@ func (h *AppendableHistory) appendFailed(request EtcdRequest, start, end time.Du } func (h *AppendableHistory) append(op porcupine.Operation) { - if op.Return != -1 && op.Call >= op.Return { + if op.Call >= op.Return { panic(fmt.Sprintf("Invalid operation, call(%d) >= return(%d)", op.Call, op.Return)) } if len(h.operations) > 0 { @@ -488,36 +486,10 @@ func (h History) Len() int { func (h History) Operations() []porcupine.Operation { operations := make([]porcupine.Operation, 0, len(h.operations)) - maxTime := h.lastObservedTime() - for _, op := range h.operations { - // Failed requests don't have a known return time. - if op.Return == -1 { - // Simulate Infinity by using last observed time. - op.Return = maxTime + time.Second.Nanoseconds() - } - operations = append(operations, op) - } + operations = append(operations, h.operations...) return operations } -func (h History) lastObservedTime() int64 { - var maxTime int64 - for _, op := range h.operations { - if op.Return == -1 { - // Collect call time from failed operations - if op.Call > maxTime { - maxTime = op.Call - } - } else { - // Collect return time from successful operations - if op.Return > maxTime { - maxTime = op.Return - } - } - } - return maxTime -} - func (h History) MaxRevision() int64 { var maxRevision int64 for _, op := range h.operations { diff --git a/tests/robustness/validate/operations.go b/tests/robustness/validate/operations.go index 605df732c617..2fbe69b7e281 100644 --- a/tests/robustness/validate/operations.go +++ b/tests/robustness/validate/operations.go @@ -17,6 +17,7 @@ package validate import ( "errors" "fmt" + "math" "time" "github.com/anishathalye/porcupine" @@ -53,6 +54,8 @@ func validateLinearizableOperationsAndVisualize( operations []porcupine.Operation, timeout time.Duration, ) (results Results) { + operations = patchResponseTimeOfFailedOperations(operations) + lg.Info("Validating linearizable operations", zap.Duration("timeout", timeout)) start := time.Now() result, info := porcupine.CheckOperationsVerbose(model.NonDeterministicModel, operations, timeout) @@ -75,6 +78,20 @@ func validateLinearizableOperationsAndVisualize( } } +// Failed writes can still be persisted, but we don't know when request has took effect, so we override it with MaxInt64 +func patchResponseTimeOfFailedOperations(operations []porcupine.Operation) []porcupine.Operation { + patchedOperations := make([]porcupine.Operation, 0, len(operations)) + for _, op := range operations { + request := op.Input.(model.EtcdRequest) + resp := op.Output.(model.MaybeEtcdResponse) + if resp.Error != "" && !request.IsRead() { + op.Return = math.MaxInt64 + patchedOperations = append(patchedOperations, op) + } + } + return patchedOperations +} + func validateSerializableOperations(lg *zap.Logger, operations []porcupine.Operation, replay *model.EtcdReplay) (lastErr error) { lg.Info("Validating serializable operations") for _, read := range operations { diff --git a/tests/robustness/validate/patch_history.go b/tests/robustness/validate/patch_history.go index b4f6787db958..cec98184d1e0 100644 --- a/tests/robustness/validate/patch_history.go +++ b/tests/robustness/validate/patch_history.go @@ -28,7 +28,7 @@ func patchLinearizableOperations(reports []report.ClientReport, persistedRequest putRevision := putRevision(reports) persistedPutCount := countPersistedPuts(persistedRequests) clientPutCount := countClientPuts(reports) - putReturnTime := uniquePutReturnTime(allOperations, reports, persistedRequests, clientPutCount) + putReturnTime := uniquePutReturnTime(allOperations, reports, clientPutCount) return patchOperations(allOperations, putRevision, putReturnTime, clientPutCount, persistedPutCount) } @@ -96,7 +96,7 @@ func patchOperations(operations []porcupine.Operation, watchRevision, putReturnT txnRevision = revision } if returnTime, ok := putReturnTime[kv]; ok { - op.Return = min(op.Return, returnTime) + op.Return = returnTime } case model.DeleteOperation: case model.RangeOperation: @@ -110,9 +110,9 @@ func patchOperations(operations []porcupine.Operation, watchRevision, putReturnT continue } if txnRevision != 0 { - op.Output = model.MaybeEtcdResponse{Persisted: true, PersistedRevision: txnRevision} + op.Output = model.MaybeEtcdResponse{Persisted: true, PersistedRevision: txnRevision, Error: resp.Error} } else { - op.Output = model.MaybeEtcdResponse{Persisted: true} + op.Output = model.MaybeEtcdResponse{Persisted: true, Error: resp.Error} } } // Leave operation as it is as we cannot discard it. @@ -155,11 +155,13 @@ func hasUniqueWriteOperation(ops []model.EtcdOperation, clientRequestCount map[k return false } -func uniquePutReturnTime(allOperations []porcupine.Operation, reports []report.ClientReport, persistedRequests []model.EtcdRequest, clientPutCount map[keyValue]int64) map[keyValue]int64 { +func uniquePutReturnTime(allOperations []porcupine.Operation, reports []report.ClientReport, clientPutCount map[keyValue]int64) map[keyValue]int64 { earliestReturnTime := map[keyValue]int64{} - var lastReturnTime int64 + failedRequest := map[keyValue]bool{} + for _, op := range allOperations { request := op.Input.(model.EtcdRequest) + resp := op.Output.(model.MaybeEtcdResponse) switch request.Type { case model.Txn: for _, etcdOp := range append(request.Txn.OperationsOnSuccess, request.Txn.OperationsOnFailure...) { @@ -170,10 +172,14 @@ func uniquePutReturnTime(allOperations []porcupine.Operation, reports []report.C if count := clientPutCount[kv]; count > 1 { continue } + if resp.Error != "" { + failedRequest[kv] = true + } if returnTime, ok := earliestReturnTime[kv]; !ok || returnTime > op.Return { + // The time that the response is received + // This doesn't mean anything for the failed request, as the request might have actually succeeded (e.g. connection dropped, etc.) earliestReturnTime[kv] = op.Return } - earliestReturnTime[kv] = op.Return } case model.Range: case model.LeaseGrant: @@ -182,11 +188,11 @@ func uniquePutReturnTime(allOperations []porcupine.Operation, reports []report.C default: panic(fmt.Sprintf("Unknown request type: %q", request.Type)) } - if op.Return > lastReturnTime { - lastReturnTime = op.Return - } } + // for failed requests, we use the time that the watch is received as the return time + // since the values returned by the watch are the values that are actually written in the database + // notice that the time that the value is sent by the watch might be earlier or later than the response received time due to issues such as network problems for _, client := range reports { for _, watch := range client.Watch { for _, resp := range watch.Responses { @@ -198,8 +204,12 @@ func uniquePutReturnTime(allOperations []porcupine.Operation, reports []report.C if count := clientPutCount[kv]; count > 1 { continue } - if t, ok := earliestReturnTime[kv]; !ok || t > resp.Time.Nanoseconds() { + if _, ok := failedRequest[kv]; ok { earliestReturnTime[kv] = resp.Time.Nanoseconds() + } else { + if t, ok := earliestReturnTime[kv]; !ok || t > resp.Time.Nanoseconds() { + earliestReturnTime[kv] = resp.Time.Nanoseconds() + } } case model.DeleteOperation: default: @@ -210,32 +220,6 @@ func uniquePutReturnTime(allOperations []porcupine.Operation, reports []report.C } } - for i := len(persistedRequests) - 1; i >= 0; i-- { - request := persistedRequests[i] - switch request.Type { - case model.Txn: - lastReturnTime-- - for _, op := range request.Txn.OperationsOnSuccess { - if op.Type != model.PutOperation { - continue - } - kv := keyValue{Key: op.Put.Key, Value: op.Put.Value} - if count := clientPutCount[kv]; count > 1 { - continue - } - returnTime, ok := earliestReturnTime[kv] - if ok { - lastReturnTime = min(returnTime, lastReturnTime) - earliestReturnTime[kv] = lastReturnTime - } - } - case model.LeaseGrant: - case model.LeaseRevoke: - case model.Compact: - default: - panic(fmt.Sprintf("Unknown request type: %q", request.Type)) - } - } return earliestReturnTime } diff --git a/tests/robustness/validate/patch_history_test.go b/tests/robustness/validate/patch_history_test.go index b5611d602373..c07cfaa81b49 100644 --- a/tests/robustness/validate/patch_history_test.go +++ b/tests/robustness/validate/patch_history_test.go @@ -70,13 +70,13 @@ func TestPatchHistory(t *testing.T) { putRequest("key", "value"), }, expectedRemainingOperations: []porcupine.Operation{ - {Return: infinite + 99, Output: model.MaybeEtcdResponse{Persisted: true}}, + {Return: infinite, Output: model.MaybeEtcdResponse{Persisted: true, Error: "failed"}}, }, }, { name: "failed put remains if there is a matching event, uniqueness allows for return time to be based on next persisted request", historyFunc: func(h *model.AppendableHistory) { - h.AppendPut("key1", "value", 100, infinite, nil, errors.New("failed")) + h.AppendPut("key1", "value", 100, 299, nil, errors.New("failed")) h.AppendPut("key2", "value", 300, 400, &clientv3.PutResponse{}, nil) }, persistedRequest: []model.EtcdRequest{ @@ -84,7 +84,7 @@ func TestPatchHistory(t *testing.T) { putRequest("key2", "value"), }, expectedRemainingOperations: []porcupine.Operation{ - {Return: 399, Output: model.MaybeEtcdResponse{Persisted: true}}, + {Return: 299, Output: model.MaybeEtcdResponse{Persisted: true, Error: "failed"}}, {Return: 400, Output: putResponse(model.EtcdOperationResult{})}, }, }, @@ -98,7 +98,7 @@ func TestPatchHistory(t *testing.T) { }, watchOperations: watchResponse(300, putEvent("key", "value", 2)), expectedRemainingOperations: []porcupine.Operation{ - {Return: 300, Output: model.MaybeEtcdResponse{Persisted: true, PersistedRevision: 2}}, + {Return: 300, Output: model.MaybeEtcdResponse{Persisted: true, PersistedRevision: 2, Error: "failed"}}, }, }, { @@ -113,7 +113,7 @@ func TestPatchHistory(t *testing.T) { }, watchOperations: watchResponse(3, putEvent("key", "value", 2), putEvent("key", "value", 3)), expectedRemainingOperations: []porcupine.Operation{ - {Return: 1000000004, Output: model.MaybeEtcdResponse{Error: "failed"}}, + {Return: 2, Output: model.MaybeEtcdResponse{Error: "failed"}}, {Return: 4, Output: putResponse(model.EtcdOperationResult{})}, }, }, @@ -152,13 +152,13 @@ func TestPatchHistory(t *testing.T) { putRequestWithLease("key", "value", 123), }, expectedRemainingOperations: []porcupine.Operation{ - {Return: infinite + 99, Output: model.MaybeEtcdResponse{Persisted: true}}, + {Return: infinite, Output: model.MaybeEtcdResponse{Persisted: true, Error: "failed"}}, }, }, { name: "failed put with lease remains if there is a matching event, uniqueness allows return time to be based on next persisted request", historyFunc: func(h *model.AppendableHistory) { - h.AppendPutWithLease("key1", "value", 123, 100, infinite, nil, errors.New("failed")) + h.AppendPutWithLease("key1", "value", 123, 100, 299, nil, errors.New("failed")) h.AppendPutWithLease("key2", "value", 234, 300, 400, &clientv3.PutResponse{}, nil) }, persistedRequest: []model.EtcdRequest{ @@ -166,7 +166,7 @@ func TestPatchHistory(t *testing.T) { putRequestWithLease("key2", "value", 234), }, expectedRemainingOperations: []porcupine.Operation{ - {Return: 399, Output: model.MaybeEtcdResponse{Persisted: true}}, + {Return: 299, Output: model.MaybeEtcdResponse{Persisted: true, Error: "failed"}}, {Return: 400, Output: putResponse(model.EtcdOperationResult{})}, }, }, @@ -180,7 +180,7 @@ func TestPatchHistory(t *testing.T) { }, watchOperations: watchResponse(3, putEvent("key", "value", 2)), expectedRemainingOperations: []porcupine.Operation{ - {Return: 3, Output: model.MaybeEtcdResponse{Persisted: true, PersistedRevision: 2}}, + {Return: 3, Output: model.MaybeEtcdResponse{Persisted: true, PersistedRevision: 2, Error: "failed"}}, }, }, { @@ -195,7 +195,7 @@ func TestPatchHistory(t *testing.T) { }, watchOperations: watchResponse(3, putEvent("key", "value", 2), putEvent("key", "value", 3)), expectedRemainingOperations: []porcupine.Operation{ - {Return: 1000000004, Output: model.MaybeEtcdResponse{Error: "failed"}}, + {Return: 2, Output: model.MaybeEtcdResponse{Error: "failed"}}, {Return: 4, Output: putResponse(model.EtcdOperationResult{})}, }, }, @@ -225,7 +225,7 @@ func TestPatchHistory(t *testing.T) { { name: "failed delete remains, time untouched regardless of persisted event and watch", historyFunc: func(h *model.AppendableHistory) { - h.AppendDelete("key", 100, infinite, nil, errors.New("failed")) + h.AppendDelete("key", 100, 299, nil, errors.New("failed")) h.AppendPut("key", "value", 300, 400, &clientv3.PutResponse{}, nil) }, persistedRequest: []model.EtcdRequest{ @@ -233,7 +233,7 @@ func TestPatchHistory(t *testing.T) { }, watchOperations: watchResponse(3, deleteEvent("key", 2)), expectedRemainingOperations: []porcupine.Operation{ - {Return: infinite + 400, Output: model.MaybeEtcdResponse{Error: "failed"}}, + {Return: 299, Output: model.MaybeEtcdResponse{Error: "failed"}}, {Return: 400, Output: putResponse(model.EtcdOperationResult{})}, }, }, @@ -260,7 +260,7 @@ func TestPatchHistory(t *testing.T) { putRequest("key", "value"), }, expectedRemainingOperations: []porcupine.Operation{ - {Return: infinite + 99, Output: model.MaybeEtcdResponse{Persisted: true}}, + {Return: infinite, Output: model.MaybeEtcdResponse{Persisted: true, Error: "failed"}}, }, }, { @@ -269,7 +269,7 @@ func TestPatchHistory(t *testing.T) { h.AppendTxn(nil, []clientv3.Op{clientv3.OpDelete("key")}, []clientv3.Op{}, 100, infinite, nil, errors.New("failed")) }, expectedRemainingOperations: []porcupine.Operation{ - {Return: infinite + 100, Output: model.MaybeEtcdResponse{Error: "failed"}}, + {Return: infinite, Output: model.MaybeEtcdResponse{Error: "failed"}}, }, }, { @@ -287,7 +287,7 @@ func TestPatchHistory(t *testing.T) { h.AppendTxn(nil, []clientv3.Op{clientv3.OpPut("key", "value")}, []clientv3.Op{clientv3.OpDelete("key")}, 100, infinite, nil, errors.New("failed")) }, expectedRemainingOperations: []porcupine.Operation{ - {Return: infinite + 100, Output: model.MaybeEtcdResponse{Error: "failed"}}, + {Return: infinite, Output: model.MaybeEtcdResponse{Error: "failed"}}, }, }, { @@ -296,7 +296,7 @@ func TestPatchHistory(t *testing.T) { h.AppendTxn(nil, []clientv3.Op{clientv3.OpDelete("key")}, []clientv3.Op{clientv3.OpPut("key", "value")}, 100, infinite, nil, errors.New("failed")) }, expectedRemainingOperations: []porcupine.Operation{ - {Return: infinite + 100, Output: model.MaybeEtcdResponse{Error: "failed"}}, + {Return: infinite, Output: model.MaybeEtcdResponse{Error: "failed"}}, }, }, { @@ -315,13 +315,13 @@ func TestPatchHistory(t *testing.T) { putRequest("key", "value"), }, expectedRemainingOperations: []porcupine.Operation{ - {Return: infinite + 99, Output: model.MaybeEtcdResponse{Persisted: true}}, + {Return: infinite, Output: model.MaybeEtcdResponse{Persisted: true, Error: "failed"}}, }, }, { name: "failed put remains if there is a matching persisted request, uniqueness of this operation and following operation allows patching based on following operation", historyFunc: func(h *model.AppendableHistory) { - h.AppendPut("key1", "value1", 300, infinite, nil, errors.New("failed")) + h.AppendPut("key1", "value1", 300, 499, nil, errors.New("failed")) h.AppendPut("key2", "value2", 500, 600, &clientv3.PutResponse{}, nil) }, persistedRequest: []model.EtcdRequest{ @@ -329,7 +329,7 @@ func TestPatchHistory(t *testing.T) { putRequest("key2", "value2"), }, expectedRemainingOperations: []porcupine.Operation{ - {Return: 599, Output: model.MaybeEtcdResponse{Persisted: true}}, + {Return: 499, Output: model.MaybeEtcdResponse{Persisted: true, Error: "failed"}}, {Return: 600, Output: putResponse(model.EtcdOperationResult{})}, }, }, @@ -337,7 +337,7 @@ func TestPatchHistory(t *testing.T) { name: "failed put remains if there is a matching persisted request, lack of uniqueness of this operation prevents patching based on following operation", historyFunc: func(h *model.AppendableHistory) { h.AppendPut("key1", "value1", 100, 200, &clientv3.PutResponse{}, nil) - h.AppendPut("key1", "value1", 300, infinite, nil, errors.New("failed")) + h.AppendPut("key1", "value1", 300, 499, nil, errors.New("failed")) h.AppendPut("key2", "value2", 500, 600, &clientv3.PutResponse{}, nil) }, persistedRequest: []model.EtcdRequest{ @@ -347,7 +347,7 @@ func TestPatchHistory(t *testing.T) { }, expectedRemainingOperations: []porcupine.Operation{ {Return: 200, Output: putResponse(model.EtcdOperationResult{})}, - {Return: infinite + 600, Output: model.MaybeEtcdResponse{Error: "failed"}}, + {Return: 499, Output: model.MaybeEtcdResponse{Error: "failed"}}, {Return: 600, Output: putResponse(model.EtcdOperationResult{})}, }, }, @@ -355,7 +355,7 @@ func TestPatchHistory(t *testing.T) { name: "failed put remains if there is a matching persisted request, lack of uniqueness of following operation prevents patching based on following operation", historyFunc: func(h *model.AppendableHistory) { h.AppendPut("key2", "value2", 100, 200, &clientv3.PutResponse{}, nil) - h.AppendPut("key1", "value1", 300, infinite, nil, errors.New("failed")) + h.AppendPut("key1", "value1", 300, 499, nil, errors.New("failed")) h.AppendPut("key2", "value2", 500, 600, &clientv3.PutResponse{}, nil) }, persistedRequest: []model.EtcdRequest{ @@ -366,7 +366,7 @@ func TestPatchHistory(t *testing.T) { expectedRemainingOperations: []porcupine.Operation{ {Return: 200, Output: putResponse(model.EtcdOperationResult{})}, // TODO: We can infer that failed operation finished before last operation matching following. - {Return: infinite + 598, Output: model.MaybeEtcdResponse{Persisted: true}}, + {Return: 499, Output: model.MaybeEtcdResponse{Persisted: true, Error: "failed"}}, {Return: 600, Output: putResponse(model.EtcdOperationResult{})}, }, }, @@ -376,7 +376,7 @@ func TestPatchHistory(t *testing.T) { h.AppendTxn(nil, []clientv3.Op{}, []clientv3.Op{clientv3.OpDelete("key")}, 100, infinite, nil, errors.New("failed")) }, expectedRemainingOperations: []porcupine.Operation{ - {Return: infinite + 100, Output: model.MaybeEtcdResponse{Error: "failed"}}, + {Return: infinite, Output: model.MaybeEtcdResponse{Error: "failed"}}, }, }, {