From d5a6d2518d43516f43184efc5d1341a2a2072355 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Sun, 30 Oct 2022 03:49:01 -0500 Subject: [PATCH 1/2] tests: Optimize checking failed writes Signed-off-by: Marek Siarkowicz --- tests/linearizability/model.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/linearizability/model.go b/tests/linearizability/model.go index 389f2fd8953..eebb9c05183 100644 --- a/tests/linearizability/model.go +++ b/tests/linearizability/model.go @@ -101,12 +101,11 @@ func step(state EtcdState, request etcdRequest, response etcdResponse) (bool, Et if state.Value == response.getData { return true, state } - for write := range state.FailedWrites { - if write == response.getData { - state.Value = response.getData - delete(state.FailedWrites, write) - return true, state - } + _, ok := state.FailedWrites[response.getData] + if ok { + state.Value = response.getData + delete(state.FailedWrites, response.getData) + return true, state } case Put: if response.err == nil { From ff6c93f6302071b8e635c46e381858b6c603ca90 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Sun, 30 Oct 2022 04:18:05 -0500 Subject: [PATCH 2/2] tests: Add revision to etcd linearizability model Signed-off-by: Marek Siarkowicz --- tests/linearizability/client.go | 10 ++- tests/linearizability/model.go | 78 +++++++++++++------ tests/linearizability/model_test.go | 116 +++++++++++++++++++--------- 3 files changed, 142 insertions(+), 62 deletions(-) diff --git a/tests/linearizability/client.go b/tests/linearizability/client.go index 1b3f9f72ef4..5addf729769 100644 --- a/tests/linearizability/client.go +++ b/tests/linearizability/client.go @@ -66,7 +66,7 @@ func (c *recordingClient) Get(ctx context.Context, key string) error { ClientId: c.id, Input: etcdRequest{op: Get, key: key}, Call: callTime.UnixNano(), - Output: etcdResponse{getData: readData}, + Output: etcdResponse{getData: readData, revision: resp.Header.Revision}, Return: returnTime.UnixNano(), }) return nil @@ -74,13 +74,17 @@ func (c *recordingClient) Get(ctx context.Context, key string) error { func (c *recordingClient) Put(ctx context.Context, key, value string) error { callTime := time.Now() - _, err := c.client.Put(ctx, key, value) + resp, err := c.client.Put(ctx, key, value) returnTime := time.Now() + var revision int64 + if resp != nil && resp.Header != nil { + revision = resp.Header.Revision + } c.operations = append(c.operations, porcupine.Operation{ ClientId: c.id, Input: etcdRequest{op: Put, key: key, putData: value}, Call: callTime.UnixNano(), - Output: etcdResponse{err: err}, + Output: etcdResponse{err: err, revision: revision}, Return: returnTime.UnixNano(), }) return nil diff --git a/tests/linearizability/model.go b/tests/linearizability/model.go index eebb9c05183..5a9000f6962 100644 --- a/tests/linearizability/model.go +++ b/tests/linearizability/model.go @@ -33,13 +33,15 @@ type etcdRequest struct { } type etcdResponse struct { - getData string - err error + getData string + revision int64 + err error } type EtcdState struct { Key string Value string + LastRevision int64 FailedWrites map[string]struct{} } @@ -51,9 +53,6 @@ var etcdModel = porcupine.Model{ if err != nil { panic(err) } - if state.FailedWrites == nil { - state.FailedWrites = map[string]struct{}{} - } ok, state := step(state, in.(etcdRequest), out.(etcdResponse)) data, err := json.Marshal(state) if err != nil { @@ -64,22 +63,19 @@ var etcdModel = porcupine.Model{ DescribeOperation: func(in, out interface{}) string { request := in.(etcdRequest) response := out.(etcdResponse) - var resp string switch request.op { case Get: if response.err != nil { - resp = response.err.Error() + return fmt.Sprintf("get(%q) -> %q", request.key, response.err) } else { - resp = response.getData + return fmt.Sprintf("get(%q) -> %q, rev: %d", request.key, response.getData, response.revision) } - return fmt.Sprintf("get(%q) -> %q", request.key, resp) case Put: if response.err != nil { - resp = response.err.Error() + return fmt.Sprintf("put(%q, %q) -> %s", request.key, request.putData, response.err) } else { - resp = "ok" + return fmt.Sprintf("put(%q, %q) -> ok, rev: %d", request.key, request.putData, response.revision) } - return fmt.Sprintf("put(%q, %q) -> %s", request.key, request.putData, resp) default: return "" } @@ -88,32 +84,68 @@ var etcdModel = porcupine.Model{ func step(state EtcdState, request etcdRequest, response etcdResponse) (bool, EtcdState) { if request.key == "" { - panic("Invalid request") + panic("invalid request") } if state.Key == "" { - state.Key = request.key + return true, initState(request, response) } if state.Key != request.key { panic("Multiple keys not supported") } switch request.op { case Get: - if state.Value == response.getData { - return true, state - } - _, ok := state.FailedWrites[response.getData] - if ok { - state.Value = response.getData - delete(state.FailedWrites, response.getData) - return true, state - } + return stepGet(state, request, response) + case Put: + return stepPut(state, request, response) + default: + panic("Unknown operation") + } +} + +func initState(request etcdRequest, response etcdResponse) EtcdState { + state := EtcdState{ + Key: request.key, + LastRevision: response.revision, + FailedWrites: map[string]struct{}{}, + } + switch request.op { + case Get: + state.Value = response.getData case Put: if response.err == nil { state.Value = request.putData } else { state.FailedWrites[request.putData] = struct{}{} } + default: + panic("Unknown operation") + } + return state +} + +func stepGet(state EtcdState, request etcdRequest, response etcdResponse) (bool, EtcdState) { + if state.Value == response.getData && state.LastRevision <= response.revision { + return true, state + } + _, ok := state.FailedWrites[response.getData] + if ok && state.LastRevision < response.revision { + state.Value = response.getData + state.LastRevision = response.revision + delete(state.FailedWrites, response.getData) return true, state } return false, state } + +func stepPut(state EtcdState, request etcdRequest, response etcdResponse) (bool, EtcdState) { + if response.err != nil { + state.FailedWrites[request.putData] = struct{}{} + return true, state + } + if state.LastRevision >= response.revision { + return false, state + } + state.Value = request.putData + state.LastRevision = response.revision + return true, state +} diff --git a/tests/linearizability/model_test.go b/tests/linearizability/model_test.go index 1e29b070cb3..61453310a98 100644 --- a/tests/linearizability/model_test.go +++ b/tests/linearizability/model_test.go @@ -16,68 +16,112 @@ package linearizability import ( "errors" - "github.com/anishathalye/porcupine" "testing" ) func TestModel(t *testing.T) { tcs := []struct { - name string - okOperations []porcupine.Operation - failOperation *porcupine.Operation + name string + operations []testOperation }{ { - name: "Etcd must return what was written", - okOperations: []porcupine.Operation{ - {Input: etcdRequest{op: Put, key: "key", putData: "1"}, Output: etcdResponse{}}, - {Input: etcdRequest{op: Get, key: "key"}, Output: etcdResponse{getData: "1"}}, + name: "First Get can start from non-empty value and non-zero revision", + operations: []testOperation{ + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{getData: "2", revision: 42}}, }, - failOperation: &porcupine.Operation{Input: etcdRequest{op: Get, key: "key"}, Output: etcdResponse{getData: "2"}}, }, { - name: "Etcd can crash after storing result but before returning success to client", - okOperations: []porcupine.Operation{ - {Input: etcdRequest{op: Put, key: "key", putData: "1"}, Output: etcdResponse{err: errors.New("failed")}}, - {Input: etcdRequest{op: Get, key: "key"}, Output: etcdResponse{getData: "1"}}, + name: "First Put can start from non-zero revision", + operations: []testOperation{ + {req: etcdRequest{op: Put, key: "key", putData: "2"}, resp: etcdResponse{revision: 42}}, }, }, { - name: "Etcd can crash before storing result", - okOperations: []porcupine.Operation{ - {Input: etcdRequest{op: Put, key: "key", putData: "1"}, Output: etcdResponse{err: errors.New("failed")}}, - {Input: etcdRequest{op: Get, key: "key"}, Output: etcdResponse{getData: ""}}, + name: "Get response data should match PUT", + operations: []testOperation{ + {req: etcdRequest{op: Put, key: "key", putData: "1"}, resp: etcdResponse{revision: 1}}, + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{getData: "2", revision: 1}, failure: true}, + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{getData: "1", revision: 1}}, }, }, { - name: "Etcd can continue errored request after it failed", - okOperations: []porcupine.Operation{ - {Input: etcdRequest{op: Put, key: "key", putData: "1"}, Output: etcdResponse{err: errors.New("failed")}}, - {Input: etcdRequest{op: Get, key: "key"}, Output: etcdResponse{getData: ""}}, - {Input: etcdRequest{op: Put, key: "key"}, Output: etcdResponse{getData: "2"}}, - {Input: etcdRequest{op: Get, key: "key"}, Output: etcdResponse{getData: "1"}}, + name: "Get response revision should be equal or greater then put", + operations: []testOperation{ + {req: etcdRequest{op: Put, key: "key"}, resp: etcdResponse{revision: 2}}, + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{revision: 1}, failure: true}, + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{revision: 2}}, + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{revision: 4}}, + }, + }, + { + name: "Put bumps revision", + operations: []testOperation{ + {req: etcdRequest{op: Put, key: "key", putData: "1"}, resp: etcdResponse{revision: 1}}, + {req: etcdRequest{op: Put, key: "key", putData: "2"}, resp: etcdResponse{revision: 1}, failure: true}, + {req: etcdRequest{op: Put, key: "key", putData: "2"}, resp: etcdResponse{revision: 2}}, + }, + }, + { + name: "Put can fail and be lost", + operations: []testOperation{ + {req: etcdRequest{op: Put, key: "key", putData: "1"}, resp: etcdResponse{revision: 1}}, + {req: etcdRequest{op: Put, key: "key", putData: "2"}, resp: etcdResponse{err: errors.New("failed")}}, + {req: etcdRequest{op: Put, key: "key", putData: "3"}, resp: etcdResponse{revision: 2}}, + }, + }, + { + name: "Put can fail but bump revision", + operations: []testOperation{ + {req: etcdRequest{op: Put, key: "key", putData: "1"}, resp: etcdResponse{revision: 1}}, + {req: etcdRequest{op: Put, key: "key", putData: "2"}, resp: etcdResponse{err: errors.New("failed")}}, + {req: etcdRequest{op: Put, key: "key", putData: "3"}, resp: etcdResponse{revision: 3}}, + }, + }, + { + name: "Put can fail but be persisted and bump revision", + operations: []testOperation{ + {req: etcdRequest{op: Put, key: "key", putData: "1"}, resp: etcdResponse{revision: 1}}, + {req: etcdRequest{op: Put, key: "key", putData: "2"}, resp: etcdResponse{err: errors.New("failed")}}, + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{getData: "2", revision: 1}, failure: true}, + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{getData: "2", revision: 2}}, + }, + }, + { + name: "Put can fail but be persisted later", + operations: []testOperation{ + {req: etcdRequest{op: Put, key: "key", putData: "1"}, resp: etcdResponse{err: errors.New("failed")}}, + {req: etcdRequest{op: Put, key: "key", putData: "2"}, resp: etcdResponse{revision: 2}}, + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{getData: "2", revision: 2}}, + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{getData: "1", revision: 3}}, + }, + }, + { + name: "Put can fail but bump revision later", + operations: []testOperation{ + {req: etcdRequest{op: Put, key: "key", putData: "1"}, resp: etcdResponse{err: errors.New("failed")}}, + {req: etcdRequest{op: Put, key: "key", putData: "2"}, resp: etcdResponse{revision: 2}}, + {req: etcdRequest{op: Get, key: "key"}, resp: etcdResponse{getData: "2", revision: 2}}, + {req: etcdRequest{op: Put, key: "key", putData: "3"}, resp: etcdResponse{revision: 4}}, }, - failOperation: &porcupine.Operation{Input: etcdRequest{op: Get, key: "key"}, Output: etcdResponse{getData: ""}}, }, } for _, tc := range tcs { var ok bool t.Run(tc.name, func(t *testing.T) { state := etcdModel.Init() - for _, op := range tc.okOperations { + for _, op := range tc.operations { t.Logf("state: %v", state) - ok, state = etcdModel.Step(state, op.Input, op.Output) - if !ok { - t.Errorf("Unexpected failed operation: %s", etcdModel.DescribeOperation(op.Input, op.Output)) + ok, state = etcdModel.Step(state, op.req, op.resp) + if ok != !op.failure { + t.Errorf("Unexpected operation result, expect: %v, got: %v, operation: %s", !op.failure, ok, etcdModel.DescribeOperation(op.req, op.resp)) } } - if tc.failOperation != nil { - t.Logf("state: %v", state) - ok, state = etcdModel.Step(state, tc.failOperation.Input, tc.failOperation.Output) - if ok { - t.Errorf("Unexpected succesfull operation: %s", etcdModel.DescribeOperation(tc.failOperation.Input, tc.failOperation.Output)) - } - - } }) } } + +type testOperation struct { + req etcdRequest + resp etcdResponse + failure bool +}