From de991a9f2df4d762a6da2f82aa770b9745415078 Mon Sep 17 00:00:00 2001 From: Geeta Gharpure Date: Thu, 17 Nov 2022 14:19:04 -0800 Subject: [PATCH 1/4] linearizability tests - Add support for delete api Signed-off-by: Geeta Gharpure --- tests/linearizability/client.go | 23 +++++++++++++++++++++++ tests/linearizability/model.go | 29 +++++++++++++++++++++++++++-- tests/linearizability/model_test.go | 12 ++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/tests/linearizability/client.go b/tests/linearizability/client.go index 5addf7297..fe55436ff 100644 --- a/tests/linearizability/client.go +++ b/tests/linearizability/client.go @@ -89,3 +89,26 @@ func (c *recordingClient) Put(ctx context.Context, key, value string) error { }) return nil } + +func (c *recordingClient) Delete(ctx context.Context, key string) error { + callTime := time.Now() + resp, err := c.client.Delete(ctx, key) + returnTime := time.Now() + if err != nil { + return err + } + var revision int64 + var deleted int64 + if resp != nil && resp.Header != nil { + revision = resp.Header.Revision + deleted = resp.Deleted + } + c.operations = append(c.operations, porcupine.Operation{ + ClientId: c.id, + Input: etcdRequest{op: Delete, key: key}, + Call: callTime.UnixNano(), + Output: etcdResponse{revision: revision, deleted: deleted}, + Return: returnTime.UnixNano(), + }) + return nil +} diff --git a/tests/linearizability/model.go b/tests/linearizability/model.go index a126f7cf2..fc825c014 100644 --- a/tests/linearizability/model.go +++ b/tests/linearizability/model.go @@ -24,8 +24,9 @@ import ( type Operation string const ( - Get Operation = "get" - Put Operation = "put" + Get Operation = "get" + Put Operation = "put" + Delete Operation = "delete" ) type etcdRequest struct { @@ -37,6 +38,7 @@ type etcdRequest struct { type etcdResponse struct { getData string revision int64 + deleted int64 err error } @@ -78,6 +80,12 @@ var etcdModel = porcupine.Model{ } else { return fmt.Sprintf("put(%q, %q) -> ok, rev: %d", request.key, request.putData, response.revision) } + case Delete: + if response.err != nil { + return fmt.Sprintf("delete(%q) -> %s", request.key, response.err) + } else { + return fmt.Sprintf("delete(%q) -> ok, rev: %d deleted:%d", request.key, response.revision, response.deleted) + } default: return "" } @@ -99,6 +107,8 @@ func step(state EtcdState, request etcdRequest, response etcdResponse) (bool, Et return stepGet(state, request, response) case Put: return stepPut(state, request, response) + case Delete: + return stepDelete(state, request, response) default: panic("Unknown operation") } @@ -119,6 +129,9 @@ func initState(request etcdRequest, response etcdResponse) EtcdState { } else { state.FailedWrites[request.putData] = struct{}{} } + case Delete: + //TODO should state have 'deleted' field? + state.Value = "" default: panic("Unknown operation") } @@ -151,3 +164,15 @@ func stepPut(state EtcdState, request etcdRequest, response etcdResponse) (bool, state.LastRevision = response.revision return true, state } + +func stepDelete(state EtcdState, request etcdRequest, response etcdResponse) (bool, EtcdState) { + if response.err != nil { + state.FailedWrites[request.putData] = struct{}{} + return true, state + } + if response.deleted == 1 && state.LastRevision >= response.revision { + return false, state + } + state.LastRevision = response.revision + return true, state +} diff --git a/tests/linearizability/model_test.go b/tests/linearizability/model_test.go index 61453310a..8d1a82a8c 100644 --- a/tests/linearizability/model_test.go +++ b/tests/linearizability/model_test.go @@ -104,6 +104,18 @@ func TestModel(t *testing.T) { {req: etcdRequest{op: Put, key: "key", putData: "3"}, resp: etcdResponse{revision: 4}}, }, }, + { + name: "Deleting non existent key does not change revision", + operations: []testOperation{ + {req: etcdRequest{op: Delete, key: "NotThere"}, resp: etcdResponse{deleted: 0, revision: 4}}, + }, + }, + { + name: "Deleting existent key bumps up revision", + operations: []testOperation{ + {req: etcdRequest{op: Delete, key: "key"}, resp: etcdResponse{deleted: 1, revision: 5}}, + }, + }, } for _, tc := range tcs { var ok bool From 6be22d84eff6eb131331ecb84a3f15711851f8e5 Mon Sep 17 00:00:00 2001 From: Geeta Gharpure Date: Mon, 28 Nov 2022 11:24:32 -0800 Subject: [PATCH 2/4] Updated as per review feedback. Signed-off-by: Geeta Gharpure --- tests/linearizability/model.go | 24 ++++++++++++++++++++---- tests/linearizability/model_test.go | 14 ++++++-------- tests/linearizability/traffic.go | 4 +++- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/tests/linearizability/model.go b/tests/linearizability/model.go index fc825c014..d9bbcb20b 100644 --- a/tests/linearizability/model.go +++ b/tests/linearizability/model.go @@ -130,8 +130,10 @@ func initState(request etcdRequest, response etcdResponse) EtcdState { state.FailedWrites[request.putData] = struct{}{} } case Delete: - //TODO should state have 'deleted' field? - state.Value = "" + if response.err != nil { + state.FailedWrites[""] = struct{}{} + } + //TODO preserve information about failed deletes default: panic("Unknown operation") } @@ -167,12 +169,26 @@ func stepPut(state EtcdState, request etcdRequest, response etcdResponse) (bool, func stepDelete(state EtcdState, request etcdRequest, response etcdResponse) (bool, EtcdState) { if response.err != nil { - state.FailedWrites[request.putData] = struct{}{} + state.FailedWrites[""] = struct{}{} return true, state } - if response.deleted == 1 && state.LastRevision >= response.revision { + deleteSucceeded := response.deleted != 0 + keySet := state.Value != "" + + //non-existent key cannot be deleted. + if deleteSucceeded != keySet { return false, state } + //if key was deleted, response revision should go up + if deleteSucceeded && state.LastRevision >= response.revision { + return false, state + } + //if key was not deleted, response revision should not change + if !deleteSucceeded && state.LastRevision != response.revision { + return false, state + } + + state.Value = "" state.LastRevision = response.revision return true, state } diff --git a/tests/linearizability/model_test.go b/tests/linearizability/model_test.go index 8d1a82a8c..ca65a146b 100644 --- a/tests/linearizability/model_test.go +++ b/tests/linearizability/model_test.go @@ -105,15 +105,13 @@ func TestModel(t *testing.T) { }, }, { - name: "Deleting non existent key does not change revision", + name: "Delete only increases revision on success", operations: []testOperation{ - {req: etcdRequest{op: Delete, key: "NotThere"}, resp: etcdResponse{deleted: 0, revision: 4}}, - }, - }, - { - name: "Deleting existent key bumps up revision", - operations: []testOperation{ - {req: etcdRequest{op: Delete, key: "key"}, resp: etcdResponse{deleted: 1, revision: 5}}, + {req: etcdRequest{op: Put, key: "key", putData: "1"}, resp: etcdResponse{revision: 1}}, + {req: etcdRequest{op: Delete, key: "key"}, resp: etcdResponse{deleted: 1, revision: 1}, failure: true}, + {req: etcdRequest{op: Delete, key: "key"}, resp: etcdResponse{deleted: 1, revision: 2}}, + {req: etcdRequest{op: Delete, key: "key"}, resp: etcdResponse{deleted: 0, revision: 3}, failure: true}, + {req: etcdRequest{op: Delete, key: "key"}, resp: etcdResponse{deleted: 0, revision: 2}}, }, }, } diff --git a/tests/linearizability/traffic.go b/tests/linearizability/traffic.go index 83c0e101c..f1507466e 100644 --- a/tests/linearizability/traffic.go +++ b/tests/linearizability/traffic.go @@ -24,7 +24,7 @@ import ( ) var ( - DefaultTraffic Traffic = readWriteSingleKey{key: "key", writes: []opChance{{operation: Put, chance: 100}}} + DefaultTraffic Traffic = readWriteSingleKey{key: "key", writes: []opChance{{operation: Put, chance: 90}, {operation: Delete, chance: 10}}} ) type Traffic interface { @@ -81,6 +81,8 @@ func (t readWriteSingleKey) Write(ctx context.Context, c *recordingClient, limit switch t.pickWriteOperation() { case Put: err = c.Put(putCtx, t.key, fmt.Sprintf("%d", id)) + case Delete: + err = c.Delete(putCtx, t.key) default: panic("invalid operation") } From eef1fb9246339b9604151896a92ba85dbf2935c1 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Wed, 30 Nov 2022 11:34:59 +0100 Subject: [PATCH 3/4] Record delete request errors Signed-off-by: Marek Siarkowicz --- tests/linearizability/client.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/linearizability/client.go b/tests/linearizability/client.go index fe55436ff..ca2da56df 100644 --- a/tests/linearizability/client.go +++ b/tests/linearizability/client.go @@ -94,9 +94,6 @@ func (c *recordingClient) Delete(ctx context.Context, key string) error { callTime := time.Now() resp, err := c.client.Delete(ctx, key) returnTime := time.Now() - if err != nil { - return err - } var revision int64 var deleted int64 if resp != nil && resp.Header != nil { @@ -107,7 +104,7 @@ func (c *recordingClient) Delete(ctx context.Context, key string) error { ClientId: c.id, Input: etcdRequest{op: Delete, key: key}, Call: callTime.UnixNano(), - Output: etcdResponse{revision: revision, deleted: deleted}, + Output: etcdResponse{revision: revision, deleted: deleted, err: err}, Return: returnTime.UnixNano(), }) return nil From ccf5d3c1af85d463f87c2c26635af58af6b95c59 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Wed, 30 Nov 2022 11:35:43 +0100 Subject: [PATCH 4/4] Cleanup comment Signed-off-by: Marek Siarkowicz --- tests/linearizability/model.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/linearizability/model.go b/tests/linearizability/model.go index d9bbcb20b..6c0617222 100644 --- a/tests/linearizability/model.go +++ b/tests/linearizability/model.go @@ -133,7 +133,6 @@ func initState(request etcdRequest, response etcdResponse) EtcdState { if response.err != nil { state.FailedWrites[""] = struct{}{} } - //TODO preserve information about failed deletes default: panic("Unknown operation") }