8554: improved tests

diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index d096dc69c3a9ee6be3ab9a960bc63e69ab202641..687c2fb36b7526bbbe498b86fd3c154d07ce9bd4 100644 (file)
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -399,6 +399,5 @@ func (v *AzureBlobVolume) isKeepBlock(s string) bool {
 // EmptyTrash looks for trashed blocks that exceeded trashLifetime
 // and deletes them from the volume.
 // TBD
-func (v *AzureBlobVolume) EmptyTrash() error {
-       return nil
+func (v *AzureBlobVolume) EmptyTrash() {
 }

diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 5bcab1d398fcb4ef77b58ce03f5b6a8a6e07457c..79a680d58a3efebab11467ca2f3a474d2e0d0feb 100644 (file)
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -326,6 +326,5 @@ func (v *S3Volume) translateError(err error) error {
 // EmptyTrash looks for trashed blocks that exceeded trashLifetime
 // and deletes them from the volume.
 // TBD
-func (v *S3Volume) EmptyTrash() error {
-       return nil
+func (v *S3Volume) EmptyTrash() {
 }

diff --git a/services/keepstore/volume.go b/services/keepstore/volume.go
index bec1ee6f7fc9a8b4f94a19c1b0d3a09f2a6003f4..17da54fdadbca571cae93fde18342d2776b4e3a7 100644 (file)
--- a/services/keepstore/volume.go
+++ b/services/keepstore/volume.go
@@ -207,7 +207,7 @@ type Volume interface {
 
        // EmptyTrash looks for trashed blocks that exceeded trashLifetime
        // and deletes them from the volume.
-       EmptyTrash() error
+       EmptyTrash()
 }
 
 // A VolumeManager tells callers which volumes can read, which volumes

diff --git a/services/keepstore/volume_generic_test.go b/services/keepstore/volume_generic_test.go
index 5b0016f2772239a5f200cda3a6feff567280739c..7e624298d381121239e8a2f9589eab05bc7a74cc 100644 (file)
--- a/services/keepstore/volume_generic_test.go
+++ b/services/keepstore/volume_generic_test.go
@@ -78,8 +78,8 @@ func DoGenericVolumeTests(t TB, factory TestableVolumeFactory) {
        testPutFullBlock(t, factory)
 
        testTrashUntrash(t, factory)
-       testEmptyTrashTrashLifetime3600s(t, factory)
-       testEmptyTrashTrashLifetime1s(t, factory)
+       testTrashEmptyTrashUntrash(t, factory)
+       testTrashUntrashWithEmptyTrashGoroutine(t, factory)
 }
 
 // Put a test block, get it and verify content
@@ -761,50 +761,182 @@ func testTrashUntrash(t TB, factory TestableVolumeFactory) {
        bufs.Put(buf)
 }
 
-// With large trashLifetime, perform:
-// Run emptyTrash goroutine with much smaller trashCheckInterval
-// Trash an old block - which either raises ErrNotImplemented or succeeds
-// Untrash - which either raises ErrNotImplemented or succeeds
-// Get - which must find the block
-func testEmptyTrashTrashLifetime3600s(t TB, factory TestableVolumeFactory) {
+func testTrashEmptyTrashUntrash(t TB, factory TestableVolumeFactory) {
        v := factory(t)
        defer v.Teardown()
-
-       doneEmptyingTrash := make(chan bool)
        defer func() {
                trashLifetime = 0 * time.Second
-               doneEmptyingTrash <- true
        }()
 
-       trashLifetime = 3600 * time.Second
-       trashCheckInterval = 1 * time.Second
-
-       go emptyTrash(doneEmptyingTrash, trashCheckInterval)
+       // With trashLifetime = 1h, test trash/untrash cycle.
+       trashLifetime = 1 * time.Hour
 
-       // Trash old block
-       err := trashUntrashOldBlock(t, v, 2)
+       // put block and backdate it
+       v.PutRaw(TestHash, TestBlock)
+       v.TouchWithDate(TestHash, time.Now().Add(-2*blobSignatureTTL))
 
-       // Get is expected to succeed after untrash before EmptyTrash
-       // It is still found on readonly volumes
        buf, err := v.Get(TestHash)
        if err != nil {
-               if !os.IsNotExist(err) {
-                       t.Errorf("os.IsNotExist(%v) should have been true", err)
-               }
-       } else {
-               if bytes.Compare(buf, TestBlock) != 0 {
-                       t.Errorf("Got data %+q, expected %+q", buf, TestBlock)
-               }
-               bufs.Put(buf)
+               t.Fatal(err)
+       }
+       if bytes.Compare(buf, TestBlock) != 0 {
+               t.Errorf("Got data %+q, expected %+q", buf, TestBlock)
+       }
+       bufs.Put(buf)
+
+       // Trash it
+       err = v.Trash(TestHash)
+       if err == MethodDisabledError || err == ErrNotImplemented {
+               return
+       }
+       buf, err = v.Get(TestHash)
+       if err == nil || !os.IsNotExist(err) {
+               t.Errorf("os.IsNotExist(%v) should have been true", err)
+       }
+
+       // Empty trash; the block is still within trashLifetime and hence is not emptied
+       v.EmptyTrash()
+
+       // Untrash will hence rescue it
+       err = v.Untrash(TestHash)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       // Get block will find it
+       buf, err = v.Get(TestHash)
+       if err != nil {
+               t.Fatal(err)
+       }
+       if bytes.Compare(buf, TestBlock) != 0 {
+               t.Errorf("Got data %+q, expected %+q", buf, TestBlock)
        }
+       bufs.Put(buf)
+
+       // With trashLifetime = 1ns, test trash/untrash cycle.
+       trashLifetime = 1 * time.Nanosecond
+
+       // Trash it
+       err = v.Trash(TestHash)
+       if err != nil {
+               t.Fatal(err)
+       }
+       buf, err = v.Get(TestHash)
+       if err == nil || !os.IsNotExist(err) {
+               t.Errorf("os.IsNotExist(%v) should have been true", err)
+       }
+
+       // Untrash
+       err = v.Untrash(TestHash)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       // Get block will find it
+       buf, err = v.Get(TestHash)
+       if err != nil {
+               t.Fatal(err)
+       }
+       if bytes.Compare(buf, TestBlock) != 0 {
+               t.Errorf("Got data %+q, expected %+q", buf, TestBlock)
+       }
+       bufs.Put(buf)
+
+       // Trash it again
+       err = v.Trash(TestHash)
+       if err == MethodDisabledError || err == ErrNotImplemented {
+               return
+       }
+       buf, err = v.Get(TestHash)
+       if err == nil || !os.IsNotExist(err) {
+               t.Errorf("os.IsNotExist(%v) should have been true", err)
+       }
+
+       // Empty trash will empty it
+       v.EmptyTrash()
+
+       // Untrash won't find it
+       err = v.Untrash(TestHash)
+       if err == nil || !os.IsNotExist(err) {
+               t.Errorf("os.IsNotExist(%v) should have been true", err)
+       }
+
+       // Get block won't find it
+       buf, err = v.Get(TestHash)
+       if err == nil || !os.IsNotExist(err) {
+               t.Errorf("os.IsNotExist(%v) should have been true", err)
+       }
+
+       // Still with trashLifetime = 1ns: put, trash, put one more, trash etc
+       // put block and backdate it
+       v.PutRaw(TestHash, TestBlock)
+       v.TouchWithDate(TestHash, time.Now().Add(-2*blobSignatureTTL))
+
+       // Trash
+       err = v.Trash(TestHash)
+       if err == MethodDisabledError || err == ErrNotImplemented {
+               return
+       }
+       buf, err = v.Get(TestHash)
+       if err == nil || !os.IsNotExist(err) {
+               t.Errorf("os.IsNotExist(%v) should have been true", err)
+       }
+
+       // put again
+       err = v.Put(TestHash, TestBlock)
+       if err != nil {
+               t.Fatal(err)
+       }
+       v.TouchWithDate(TestHash, time.Now().Add(-2*blobSignatureTTL))
+
+       // Empty trash will empty the trashed block but the second one is untouched
+       v.EmptyTrash()
+
+       // Get block should work because of the second block
+       buf, err = v.Get(TestHash)
+       if err != nil {
+               t.Fatal(err)
+       }
+       if bytes.Compare(buf, TestBlock) != 0 {
+               t.Errorf("Got data %+q, expected %+q", buf, TestBlock)
+       }
+       bufs.Put(buf)
+
+       // set trashLifetime to one hour
+       trashLifetime = 1 * time.Hour
+
+       // trash block
+       err = v.Trash(TestHash)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       // Empty trash won't empty this second block which is still within trashLifetime
+       v.EmptyTrash()
+
+       // Untrash; the second block which is still within trashLifetime will be rescued
+       err = v.Untrash(TestHash)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       // Get block should work because of the second block
+       buf, err = v.Get(TestHash)
+       if err != nil {
+               t.Fatal(err)
+       }
+       if bytes.Compare(buf, TestBlock) != 0 {
+               t.Errorf("Got data %+q, expected %+q", buf, TestBlock)
+       }
+       bufs.Put(buf)
 }
 
-// With trashLifetime = 1, perform:
+// With trashLifetime = 1ns, perform:
 // Run emptyTrash goroutine
 // Trash an old block - which either raises ErrNotImplemented or succeeds
 // Untrash - after emptyTrash goroutine ticks, and hence does not actually untrash
 // Get - which must fail to find the block
-func testEmptyTrashTrashLifetime1s(t TB, factory TestableVolumeFactory) {
+func testTrashUntrashWithEmptyTrashGoroutine(t TB, factory TestableVolumeFactory) {
        v := factory(t)
        defer v.Teardown()
 
@@ -816,32 +948,12 @@ func testEmptyTrashTrashLifetime1s(t TB, factory TestableVolumeFactory) {
 
        volumes = append(volumes, v)
 
-       trashLifetime = 1 * time.Second
-       trashCheckInterval = 1 * time.Second
+       trashLifetime = 1 * time.Nanosecond
+       trashCheckInterval = 1 * time.Nanosecond
 
        go emptyTrash(doneEmptyingTrash, trashCheckInterval)
 
        // Trash old block and untrash a little after first trashCheckInterval
-       err := trashUntrashOldBlock(t, v, 3)
-
-       // Get is expected to fail due to EmptyTrash before Untrash
-       // It is still found on readonly volumes
-       buf, err := v.Get(TestHash)
-       if err != nil {
-               if !os.IsNotExist(err) {
-                       t.Errorf("os.IsNotExist(%v) should have been true", err)
-               }
-       } else {
-               if bytes.Compare(buf, TestBlock) != 0 {
-                       t.Errorf("Got data %+q, expected %+q", buf, TestBlock)
-               }
-               bufs.Put(buf)
-       }
-}
-
-// Put a block, backdate it, trash it, untrash it after the untrashAfter seconds
-func trashUntrashOldBlock(t TB, v TestableVolume, untrashAfter int) error {
-       // put block and backdate it
        v.PutRaw(TestHash, TestBlock)
        v.TouchWithDate(TestHash, time.Now().Add(-2*blobSignatureTTL))
 
@@ -856,31 +968,34 @@ func trashUntrashOldBlock(t TB, v TestableVolume, untrashAfter int) error {
 
        // Trash
        err = v.Trash(TestHash)
-       if err != nil {
-               if err != ErrNotImplemented && err != MethodDisabledError {
-                       t.Fatal(err)
-               } else {
-                       // To test emptyTrash goroutine effectively, we need to give the
-                       // ticker a couple rounds, adding some sleep time to the test.
-                       // This delay is unnecessary for volumes that are currently
-                       // not yet supporting trashLifetime > 0 (this case is already
-                       // covered in the testTrashUntrash already)
-                       return err
-               }
-       } else {
-               _, err = v.Get(TestHash)
-               if err == nil || !os.IsNotExist(err) {
-                       t.Fatalf("os.IsNotExist(%v) should have been true", err)
-               }
+       if err == MethodDisabledError || err == ErrNotImplemented {
+               return
+       }
+
+       _, err = v.Get(TestHash)
+       if err == nil || !os.IsNotExist(err) {
+               t.Fatalf("os.IsNotExist(%v) should have been true", err)
        }
 
-       // Untrash after give wait time; it may have been deleted by emptyTrash goroutine
-       time.Sleep(time.Duration(untrashAfter) * time.Second)
+       time.Sleep(2 * time.Nanosecond)
+
+       // Untrash
        err = v.Untrash(TestHash)
        if err != nil {
-               if err != ErrNotImplemented && err != MethodDisabledError && err != os.ErrNotExist {
-                       t.Fatal(err)
+               t.Fatal(err)
+       }
+
+       // Get is expected to fail due to EmptyTrash before Untrash
+       // It is still found on readonly volumes
+       buf, err = v.Get(TestHash)
+       if err != nil {
+               if !os.IsNotExist(err) {
+                       t.Errorf("os.IsNotExist(%v) should have been true", err)
                }
+       } else {
+               if bytes.Compare(buf, TestBlock) != 0 {
+                       t.Errorf("Got data %+q, expected %+q", buf, TestBlock)
+               }
+               bufs.Put(buf)
        }
-       return err
 }

diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go
index 508c7fa24f4b7a385220ff1ef55e4a166ff2d9f8..e8a5a338f51cb25d47f419d02f6ca76a211d535c 100644 (file)
--- a/services/keepstore/volume_test.go
+++ b/services/keepstore/volume_test.go
@@ -224,6 +224,5 @@ func (v *MockVolume) Replication() int {
        return 1
 }
 
-func (v *MockVolume) EmptyTrash() error {
-       return nil
+func (v *MockVolume) EmptyTrash() {
 }

diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index 9183a398f34ca89cc40eb6b6c5f1c2122f831a84..3f1df64c4c672940392ad0a6470d8fd5c3e8a131 100644 (file)
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -534,7 +534,7 @@ func (v *UnixVolume) translateError(err error) error {
 
 // EmptyTrash walks hierarchy looking for {hash}.trash.*
 // and deletes those with deadline < now.
-func (v *UnixVolume) EmptyTrash() error {
+func (v *UnixVolume) EmptyTrash() {
        var bytesDeleted, bytesInTrash int64
        var blocksDeleted, blocksInTrash int
 
@@ -575,6 +575,4 @@ func (v *UnixVolume) EmptyTrash() error {
        }
 
        log.Printf("EmptyTrash stats for %v: Bytes deleted %v; Blocks deleted %v; Bytes remaining in trash: %v; Blocks remaining in trash: %v", v.String(), bytesDeleted, blocksDeleted, bytesInTrash, blocksInTrash)
-
-       return nil
 }