[ARVADOS] updated: 9c571f5fcbdf8120c30ab29b1ba3a41f52d37425

Git user git at public.curoverse.com
Wed Mar 16 12:16:19 EDT 2016


Summary of changes:
 services/keepstore/keepstore.go           |   3 -
 services/keepstore/volume_generic_test.go | 144 ++++++++++++++----------------
 services/keepstore/volume_unix.go         |   2 +
 3 files changed, 67 insertions(+), 82 deletions(-)

       via  9c571f5fcbdf8120c30ab29b1ba3a41f52d37425 (commit)
      from  f891b240e485022482c845efeffb625a31021c0d (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 9c571f5fcbdf8120c30ab29b1ba3a41f52d37425
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Mar 16 12:16:15 2016 -0400

    8554: Dedup Get() checks, add comments, fix up regexp.

diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index 9473690..40e62c5 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -11,7 +11,6 @@ import (
 	"net/http"
 	"os"
 	"os/signal"
-	"regexp"
 	"strings"
 	"syscall"
 	"time"
@@ -127,8 +126,6 @@ var (
 	volumes         volumeSet
 )
 
-var trashLocRegexp = regexp.MustCompile(`/([0-9a-f]{32}).trash.(\d+)$`)
-
 func (vs *volumeSet) String() string {
 	return fmt.Sprintf("%+v", (*vs)[:])
 }
diff --git a/services/keepstore/volume_generic_test.go b/services/keepstore/volume_generic_test.go
index c9eb096..95166c2 100644
--- a/services/keepstore/volume_generic_test.go
+++ b/services/keepstore/volume_generic_test.go
@@ -763,115 +763,104 @@ func testTrashUntrash(t TB, factory TestableVolumeFactory) {
 func testTrashEmptyTrashUntrash(t TB, factory TestableVolumeFactory) {
 	v := factory(t)
 	defer v.Teardown()
-	defer func() {
-		trashLifetime = 0 * time.Second
-	}()
+	defer func(orig time.Duration) {
+		trashLifetime = orig
+	}(trashLifetime)
+
+	checkGet := func() error {
+		buf, err := v.Get(TestHash)
+		if err != nil {
+			return err
+		}
+		if bytes.Compare(buf, TestBlock) != 0 {
+			t.Fatalf("Got data %+q, expected %+q", buf, TestBlock)
+		}
+		bufs.Put(buf)
+		return nil
+	}
 
-	// First set of tests
+	// First set: EmptyTrash before reaching the trash deadline.
 
-	// With trashLifetime = 1h, test trash/untrash cycle.
 	trashLifetime = 1 * time.Hour
 
-	// put block and backdate it
 	v.PutRaw(TestHash, TestBlock)
 	v.TouchWithDate(TestHash, time.Now().Add(-2*blobSignatureTTL))
 
-	buf, err := v.Get(TestHash)
+	err := checkGet()
 	if err != nil {
 		t.Fatal(err)
 	}
-	if bytes.Compare(buf, TestBlock) != 0 {
-		t.Fatalf("Got data %+q, expected %+q", buf, TestBlock)
-	}
-	bufs.Put(buf)
 
-	// Trash it
 	err = v.Trash(TestHash)
 	if err == MethodDisabledError || err == ErrNotImplemented {
+		// Skip the trash tests for read-only volumes, and
+		// volume types that don't support trashLifetime>0.
 		return
 	}
 
-	buf, err = v.Get(TestHash)
+	err = checkGet()
 	if err == nil || !os.IsNotExist(err) {
 		t.Fatalf("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
+	// Even after emptying the trash, we can untrash our block
+	// because the deadline hasn't been reached.
 	err = v.Untrash(TestHash)
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	// Get block will find it
-	buf, err = v.Get(TestHash)
+	err = checkGet()
 	if err != nil {
 		t.Fatal(err)
 	}
-	if bytes.Compare(buf, TestBlock) != 0 {
-		t.Fatalf("Got data %+q, expected %+q", buf, TestBlock)
-	}
-	bufs.Put(buf)
 
-	// Untrash again; should fail
+	// Untrash should fail if the only block in the trash has
+	// already been untrashed.
 	err = v.Untrash(TestHash)
 	if err == nil || !os.IsNotExist(err) {
 		t.Fatalf("os.IsNotExist(%v) should have been true", err)
 	}
 
-	buf, err = v.Get(TestHash)
+	// The failed Untrash should not interfere with our
+	// already-untrashed copy.
+	err = checkGet()
 	if err != nil {
 		t.Fatal(err)
 	}
-	if bytes.Compare(buf, TestBlock) != 0 {
-		t.Fatalf("Got data %+q, expected %+q", buf, TestBlock)
-	}
-	bufs.Put(buf)
 
-	// Second set of tests
+	// Second set: EmptyTrash after the trash deadline has passed.
 
-	// 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)
+	err = checkGet()
 	if err == nil || !os.IsNotExist(err) {
 		t.Fatalf("os.IsNotExist(%v) should have been true", err)
 	}
 
-	// Untrash
+	// Even though 1ns has passed, we can untrash because we
+	// haven't called EmptyTrash yet.
 	err = v.Untrash(TestHash)
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	// Get block will find it
-	buf, err = v.Get(TestHash)
+	err = checkGet()
 	if err != nil {
 		t.Fatal(err)
 	}
-	if bytes.Compare(buf, TestBlock) != 0 {
-		t.Fatalf("Got data %+q, expected %+q", buf, TestBlock)
-	}
-	bufs.Put(buf)
 
-	// Trash it again
+	// Trash it again, and this time call EmptyTrash so it really
+	// goes away.
 	err = v.Trash(TestHash)
-	if err == MethodDisabledError || err == ErrNotImplemented {
-		return
-	}
-	buf, err = v.Get(TestHash)
+	err = checkGet()
 	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
@@ -881,73 +870,70 @@ func testTrashEmptyTrashUntrash(t TB, factory TestableVolumeFactory) {
 	}
 
 	// Get block won't find it
-	buf, err = v.Get(TestHash)
+	err = checkGet()
 	if err == nil || !os.IsNotExist(err) {
 		t.Fatalf("os.IsNotExist(%v) should have been true", err)
 	}
 
-  // Third set of tests
+	// Third set: If the same data block gets written again after
+	// being trashed, and then the trash gets emptied, the newer
+	// un-trashed copy doesn't get deleted along with it.
 
-	// 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
+	trashLifetime = time.Nanosecond
 	err = v.Trash(TestHash)
-	if err == MethodDisabledError || err == ErrNotImplemented {
-		return
+	if err != nil {
+		t.Fatal(err)
 	}
-	buf, err = v.Get(TestHash)
+	err = checkGet()
 	if err == nil || !os.IsNotExist(err) {
 		t.Fatalf("os.IsNotExist(%v) should have been true", err)
 	}
 
-	// put again
-	err = v.Put(TestHash, TestBlock)
+	v.PutRaw(TestHash, TestBlock)
+	v.TouchWithDate(TestHash, time.Now().Add(-2*blobSignatureTTL))
+
+	// EmptyTrash should not delete the untrashed copy.
+	v.EmptyTrash()
+	err = checkGet()
 	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()
+	// Fourth set: If the same data block gets trashed twice with
+	// different deadlines A and C, and then the trash is emptied
+	// at intermediate time B (A < B < C), it is still possible to
+	// untrash the block whose deadline is "C".
 
-	// Get block should work because of the second block
-	buf, err = v.Get(TestHash)
+	v.PutRaw(TestHash, TestBlock)
+	v.TouchWithDate(TestHash, time.Now().Add(-2*blobSignatureTTL))
+
+	trashLifetime = time.Nanosecond
+	err = v.Trash(TestHash)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if bytes.Compare(buf, TestBlock) != 0 {
-		t.Fatalf("Got data %+q, expected %+q", buf, TestBlock)
-	}
-	bufs.Put(buf)
 
-	// set trashLifetime to one hour
-	trashLifetime = 1 * time.Hour
+	v.PutRaw(TestHash, TestBlock)
+	v.TouchWithDate(TestHash, time.Now().Add(-2*blobSignatureTTL))
 
-	// trash block
+	trashLifetime = time.Hour
 	err = v.Trash(TestHash)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	// Empty trash won't empty this second block which is still within trashLifetime
+	// EmptyTrash should not prevent us from recovering the
+	// time.Hour ("C") trash
 	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)
+	err = checkGet()
 	if err != nil {
 		t.Fatal(err)
 	}
-	if bytes.Compare(buf, TestBlock) != 0 {
-		t.Fatalf("Got data %+q, expected %+q", buf, TestBlock)
-	}
-	bufs.Put(buf)
 }
diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index fe221c3..ba9f27e 100644
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -540,6 +540,8 @@ func (v *UnixVolume) translateError(err error) error {
 	}
 }
 
+var trashLocRegexp = regexp.MustCompile(`/([0-9a-f]{32})\.trash\.(\d+)$`)
+
 // EmptyTrash walks hierarchy looking for {hash}.trash.*
 // and deletes those with deadline < now.
 func (v *UnixVolume) EmptyTrash() {

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list