From fa6e16971045bf148bf9ad0db0e06f5ad05216f3 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 18 Jul 2024 20:14:36 -0400 Subject: [PATCH 1/2] testing: demonstrate expected isolation properties Extend the unit tests to cover the expected isolation levels for avoiding dirty reads, non-repeatable reads, and phantom reads. These tests are intended to be demonstrative rather than exhaustive. They provide a guide for developers as to what behaviors to expect from applications consuming the library rather than robustly verifying the behavior under load and high concurrency. That is, they can show that an implementation is grossly incorrect but cannot show that an implementation is correct. --- isolation_test.go | 346 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 346 insertions(+) create mode 100644 isolation_test.go diff --git a/isolation_test.go b/isolation_test.go new file mode 100644 index 0000000..c58df59 --- /dev/null +++ b/isolation_test.go @@ -0,0 +1,346 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package memdb + +import ( + "testing" +) + +func TestMemDB_Isolation(t *testing.T) { + + id1 := "object-one" + id2 := "object-two" + id3 := "object-three" + + setup := func(t *testing.T) *MemDB { + t.Helper() + + db, err := NewMemDB(testValidSchema()) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Add two objects (with a gap between their IDs) + obj1a := testObj() + obj1a.ID = id1 + txn := db.Txn(true) + txn.Insert("main", obj1a) + + obj3 := testObj() + obj3.ID = id3 + txn.Insert("main", obj3) + txn.Commit() + return db + } + + t.Run("snapshot dirty read", func(t *testing.T) { + db := setup(t) + db2 := db.Snapshot() + + // Update an object + obj1b := testObj() + obj1b.ID = id1 + txn1 := db.Txn(true) + obj1b.Baz = "nope" + txn1.Insert("main", obj1b) + + // Insert an object + obj2 := testObj() + obj2.ID = id2 + txn1.Insert("main", obj2) + + txn2 := db2.Txn(false) + out, err := txn2.First("main", "id", id1) + if err != nil { + t.Fatalf("err: %v", err) + } + if out == nil { + t.Fatalf("should exist") + } + if out.(*TestObject).Baz == "nope" { + t.Fatalf("read from snapshot should not observe uncommitted update (dirty read)") + } + + out, err = txn2.First("main", "id", id2) + if err != nil { + t.Fatalf("err: %v", err) + } + if out != nil { + t.Fatalf("read from snapshot should not observe uncommitted insert (dirty read)") + } + + // New snapshot should not observe uncommitted writes + db3 := db.Snapshot() + txn3 := db3.Txn(false) + out, err = txn3.First("main", "id", id1) + if err != nil { + t.Fatalf("err: %v", err) + } + if out == nil { + t.Fatalf("should exist") + } + if out.(*TestObject).Baz == "nope" { + t.Fatalf("read from new snapshot should not observe uncommitted writes") + } + }) + + t.Run("transaction dirty read", func(t *testing.T) { + db := setup(t) + + // Update an object + obj1b := testObj() + obj1b.ID = id1 + txn1 := db.Txn(true) + obj1b.Baz = "nope" + txn1.Insert("main", obj1b) + + // Insert an object + obj2 := testObj() + obj2.ID = id2 + txn1.Insert("main", obj2) + + txn2 := db.Txn(false) + out, err := txn2.First("main", "id", id1) + if err != nil { + t.Fatalf("err: %v", err) + } + if out == nil { + t.Fatalf("should exist") + } + if out.(*TestObject).Baz == "nope" { + t.Fatalf("read from transaction should not observe uncommitted update (dirty read)") + } + + out, err = txn2.First("main", "id", id2) + if err != nil { + t.Fatalf("err: %v", err) + } + if out != nil { + t.Fatalf("read from transaction should not observe uncommitted insert (dirty read)") + } + }) + + t.Run("snapshot non-repeatable read", func(t *testing.T) { + db := setup(t) + db2 := db.Snapshot() + + // Update an object + obj1b := testObj() + obj1b.ID = id1 + txn1 := db.Txn(true) + obj1b.Baz = "nope" + txn1.Insert("main", obj1b) + + // Insert an object + obj2 := testObj() + obj2.ID = id3 + txn1.Insert("main", obj2) + + // Commit + txn1.Commit() + + txn2 := db2.Txn(false) + out, err := txn2.First("main", "id", id1) + if err != nil { + t.Fatalf("err: %v", err) + } + if out == nil { + t.Fatalf("should exist") + } + if out.(*TestObject).Baz == "nope" { + t.Fatalf("read from snapshot should not observe committed write from another transaction (non-repeatable read)") + } + + out, err = txn2.First("main", "id", id2) + if err != nil { + t.Fatalf("err: %v", err) + } + if out != nil { + t.Fatalf("read from snapshot should not observe committed write from another transaction (non-repeatable read)") + } + + }) + + t.Run("transaction non-repeatable read", func(t *testing.T) { + db := setup(t) + + // Update an object + obj1b := testObj() + obj1b.ID = id1 + txn1 := db.Txn(true) + obj1b.Baz = "nope" + txn1.Insert("main", obj1b) + + // Insert an object + obj2 := testObj() + obj2.ID = id3 + txn1.Insert("main", obj2) + + txn2 := db.Txn(false) + + // Commit + txn1.Commit() + + out, err := txn2.First("main", "id", id1) + if err != nil { + t.Fatalf("err: %v", err) + } + if out == nil { + t.Fatalf("should exist") + } + if out.(*TestObject).Baz == "nope" { + t.Fatalf("read from transaction should not observe committed write from another transaction (non-repeatable read)") + } + + out, err = txn2.First("main", "id", id2) + if err != nil { + t.Fatalf("err: %v", err) + } + if out != nil { + t.Fatalf("read from transaction should not observe committed write from another transaction (non-repeatable read)") + } + + }) + + t.Run("snapshot phantom read", func(t *testing.T) { + db := setup(t) + db2 := db.Snapshot() + + txn2 := db2.Txn(false) + iter, err := txn2.Get("main", "id_prefix", "object") + if err != nil { + t.Fatalf("err: %v", err) + } + out := iter.Next() + if out == nil || out.(*TestObject).ID != id1 { + t.Fatal("missing expected object 'object-one'") + } + + // Insert an object and commit + txn1 := db.Txn(true) + obj2 := testObj() + obj2.ID = id2 + txn1.Insert("main", obj2) + txn1.Commit() + + out = iter.Next() + if out == nil { + t.Fatal("expected 2 objects") + } + if out.(*TestObject).ID == id2 { + t.Fatalf("read from snapshot should not observe new objects in set (phantom read)") + } + + out = iter.Next() + if out != nil { + t.Fatal("expected only 2 objects: read from snapshot should not observe new objects in set (phantom read)") + } + + // Remove an object using an outdated pointer + txn1 = db.Txn(true) + obj1, err := txn1.First("main", "id", id1) + if err != nil { + t.Fatalf("err: %v", err) + } + txn1.Delete("main", obj1) + txn1.Commit() + + iter, err = txn2.Get("main", "id_prefix", "object") + if err != nil { + t.Fatalf("err: %v", err) + } + + out = iter.Next() + if out == nil || out.(*TestObject).ID != id1 { + t.Fatal("missing expected object 'object-one': read from snapshot should not observe deletes (phantom read)") + } + out = iter.Next() + if out == nil || out.(*TestObject).ID != id3 { + t.Fatal("missing expected object 'object-three': read from snapshot should not observe deletes (phantom read)") + } + + }) + + t.Run("transaction phantom read", func(t *testing.T) { + db := setup(t) + + txn2 := db.Txn(false) + iter, err := txn2.Get("main", "id_prefix", "object") + if err != nil { + t.Fatalf("err: %v", err) + } + out := iter.Next() + if out == nil || out.(*TestObject).ID != id1 { + t.Fatal("missing expected object 'object-one'") + } + + // Insert an object and commit + txn1 := db.Txn(true) + obj2 := testObj() + obj2.ID = id2 + txn1.Insert("main", obj2) + txn1.Commit() + + out = iter.Next() + if out == nil { + t.Fatal("expected 2 objects") + } + if out.(*TestObject).ID == id2 { + t.Fatalf("read from transaction should not observe new objects in set (phantom read)") + } + + out = iter.Next() + if out != nil { + t.Fatal("expected only 2 objects: read from transaction should not observe new objects in set (phantom read)") + } + + // Remove an object using an outdated pointer + txn1 = db.Txn(true) + obj1, err := txn1.First("main", "id", id1) + if err != nil { + t.Fatalf("err: %v", err) + } + txn1.Delete("main", obj1) + txn1.Commit() + + iter, err = txn2.Get("main", "id_prefix", "object") + if err != nil { + t.Fatalf("err: %v", err) + } + + out = iter.Next() + if out == nil || out.(*TestObject).ID != id1 { + t.Fatal("missing expected object 'object-one': read from transaction should not observe deletes (phantom read)") + } + out = iter.Next() + if out == nil || out.(*TestObject).ID != id3 { + t.Fatal("missing expected object 'object-three': read from transaction should not observe deletes (phantom read)") + } + + }) + + t.Run("snapshot commits are unobservable", func(t *testing.T) { + db := setup(t) + db2 := db.Snapshot() + + txn2 := db2.Txn(true) + obj1 := testObj() + obj1.ID = id1 + obj1.Baz = "also" + txn2.Insert("main", obj1) + txn2.Commit() + + txn1 := db.Txn(false) + out, err := txn1.First("main", "id", id1) + if err != nil { + t.Fatalf("err: %v", err) + } + if out == nil { + t.Fatalf("should exist") + } + if out.(*TestObject).Baz == "also" { + t.Fatalf("commit from snapshot should never be observed") + } + }) +} From 795ade121b6c3fe52d8d2ac92a564adc783ca468 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 23 Jul 2024 15:03:01 -0400 Subject: [PATCH 2/2] address comments on code review --- isolation_test.go | 96 ++++++++++++++++++----------------------------- 1 file changed, 36 insertions(+), 60 deletions(-) diff --git a/isolation_test.go b/isolation_test.go index c58df59..562d1f6 100644 --- a/isolation_test.go +++ b/isolation_test.go @@ -13,6 +13,12 @@ func TestMemDB_Isolation(t *testing.T) { id2 := "object-two" id3 := "object-three" + mustNoError := func(t *testing.T, err error) { + if err != nil { + t.Fatalf("unexpected test error: %v", err) + } + } + setup := func(t *testing.T) *MemDB { t.Helper() @@ -25,11 +31,11 @@ func TestMemDB_Isolation(t *testing.T) { obj1a := testObj() obj1a.ID = id1 txn := db.Txn(true) - txn.Insert("main", obj1a) + mustNoError(t, txn.Insert("main", obj1a)) obj3 := testObj() obj3.ID = id3 - txn.Insert("main", obj3) + mustNoError(t, txn.Insert("main", obj3)) txn.Commit() return db } @@ -43,18 +49,16 @@ func TestMemDB_Isolation(t *testing.T) { obj1b.ID = id1 txn1 := db.Txn(true) obj1b.Baz = "nope" - txn1.Insert("main", obj1b) + mustNoError(t, txn1.Insert("main", obj1b)) // Insert an object obj2 := testObj() obj2.ID = id2 - txn1.Insert("main", obj2) + mustNoError(t, txn1.Insert("main", obj2)) txn2 := db2.Txn(false) out, err := txn2.First("main", "id", id1) - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) if out == nil { t.Fatalf("should exist") } @@ -63,9 +67,7 @@ func TestMemDB_Isolation(t *testing.T) { } out, err = txn2.First("main", "id", id2) - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) if out != nil { t.Fatalf("read from snapshot should not observe uncommitted insert (dirty read)") } @@ -74,9 +76,7 @@ func TestMemDB_Isolation(t *testing.T) { db3 := db.Snapshot() txn3 := db3.Txn(false) out, err = txn3.First("main", "id", id1) - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) if out == nil { t.Fatalf("should exist") } @@ -93,18 +93,16 @@ func TestMemDB_Isolation(t *testing.T) { obj1b.ID = id1 txn1 := db.Txn(true) obj1b.Baz = "nope" - txn1.Insert("main", obj1b) + mustNoError(t, txn1.Insert("main", obj1b)) // Insert an object obj2 := testObj() obj2.ID = id2 - txn1.Insert("main", obj2) + mustNoError(t, txn1.Insert("main", obj2)) txn2 := db.Txn(false) out, err := txn2.First("main", "id", id1) - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) if out == nil { t.Fatalf("should exist") } @@ -113,9 +111,7 @@ func TestMemDB_Isolation(t *testing.T) { } out, err = txn2.First("main", "id", id2) - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) if out != nil { t.Fatalf("read from transaction should not observe uncommitted insert (dirty read)") } @@ -130,21 +126,19 @@ func TestMemDB_Isolation(t *testing.T) { obj1b.ID = id1 txn1 := db.Txn(true) obj1b.Baz = "nope" - txn1.Insert("main", obj1b) + mustNoError(t, txn1.Insert("main", obj1b)) // Insert an object obj2 := testObj() obj2.ID = id3 - txn1.Insert("main", obj2) + mustNoError(t, txn1.Insert("main", obj2)) // Commit txn1.Commit() txn2 := db2.Txn(false) out, err := txn2.First("main", "id", id1) - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) if out == nil { t.Fatalf("should exist") } @@ -153,9 +147,7 @@ func TestMemDB_Isolation(t *testing.T) { } out, err = txn2.First("main", "id", id2) - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) if out != nil { t.Fatalf("read from snapshot should not observe committed write from another transaction (non-repeatable read)") } @@ -170,12 +162,12 @@ func TestMemDB_Isolation(t *testing.T) { obj1b.ID = id1 txn1 := db.Txn(true) obj1b.Baz = "nope" - txn1.Insert("main", obj1b) + mustNoError(t, txn1.Insert("main", obj1b)) // Insert an object obj2 := testObj() obj2.ID = id3 - txn1.Insert("main", obj2) + mustNoError(t, txn1.Insert("main", obj2)) txn2 := db.Txn(false) @@ -183,9 +175,7 @@ func TestMemDB_Isolation(t *testing.T) { txn1.Commit() out, err := txn2.First("main", "id", id1) - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) if out == nil { t.Fatalf("should exist") } @@ -194,9 +184,7 @@ func TestMemDB_Isolation(t *testing.T) { } out, err = txn2.First("main", "id", id2) - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) if out != nil { t.Fatalf("read from transaction should not observe committed write from another transaction (non-repeatable read)") } @@ -209,9 +197,7 @@ func TestMemDB_Isolation(t *testing.T) { txn2 := db2.Txn(false) iter, err := txn2.Get("main", "id_prefix", "object") - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) out := iter.Next() if out == nil || out.(*TestObject).ID != id1 { t.Fatal("missing expected object 'object-one'") @@ -221,7 +207,7 @@ func TestMemDB_Isolation(t *testing.T) { txn1 := db.Txn(true) obj2 := testObj() obj2.ID = id2 - txn1.Insert("main", obj2) + mustNoError(t, txn1.Insert("main", obj2)) txn1.Commit() out = iter.Next() @@ -240,16 +226,12 @@ func TestMemDB_Isolation(t *testing.T) { // Remove an object using an outdated pointer txn1 = db.Txn(true) obj1, err := txn1.First("main", "id", id1) - if err != nil { - t.Fatalf("err: %v", err) - } - txn1.Delete("main", obj1) + mustNoError(t, err) + mustNoError(t, txn1.Delete("main", obj1)) txn1.Commit() iter, err = txn2.Get("main", "id_prefix", "object") - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) out = iter.Next() if out == nil || out.(*TestObject).ID != id1 { @@ -267,9 +249,7 @@ func TestMemDB_Isolation(t *testing.T) { txn2 := db.Txn(false) iter, err := txn2.Get("main", "id_prefix", "object") - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) out := iter.Next() if out == nil || out.(*TestObject).ID != id1 { t.Fatal("missing expected object 'object-one'") @@ -279,7 +259,7 @@ func TestMemDB_Isolation(t *testing.T) { txn1 := db.Txn(true) obj2 := testObj() obj2.ID = id2 - txn1.Insert("main", obj2) + mustNoError(t, txn1.Insert("main", obj2)) txn1.Commit() out = iter.Next() @@ -298,10 +278,8 @@ func TestMemDB_Isolation(t *testing.T) { // Remove an object using an outdated pointer txn1 = db.Txn(true) obj1, err := txn1.First("main", "id", id1) - if err != nil { - t.Fatalf("err: %v", err) - } - txn1.Delete("main", obj1) + mustNoError(t, err) + mustNoError(t, txn1.Delete("main", obj1)) txn1.Commit() iter, err = txn2.Get("main", "id_prefix", "object") @@ -328,14 +306,12 @@ func TestMemDB_Isolation(t *testing.T) { obj1 := testObj() obj1.ID = id1 obj1.Baz = "also" - txn2.Insert("main", obj1) + mustNoError(t, txn2.Insert("main", obj1)) txn2.Commit() txn1 := db.Txn(false) out, err := txn1.First("main", "id", id1) - if err != nil { - t.Fatalf("err: %v", err) - } + mustNoError(t, err) if out == nil { t.Fatalf("should exist") }