[ARVADOS] updated: 143a5f355100b837daa428231df0370b525a1f9f

[git at public.curoverse.com]

Summary of changes:
 services/keepstore/volume.go              |  8 +++++++-
 services/keepstore/volume_generic_test.go | 26 ++++++++++++++++++--------
 2 files changed, 25 insertions(+), 9 deletions(-)

       via  143a5f355100b837daa428231df0370b525a1f9f (commit)
      from  241f0bcdacbf83b587bff9ff45985e720bde9f0b (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 143a5f355100b837daa428231df0370b525a1f9f
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri Sep 11 16:13:53 2015 -0400

    7179: Tighten Put requirements when overwriting existing data.

diff --git a/services/keepstore/volume.go b/services/keepstore/volume.go
index e5964e2..daf003e 100644
--- a/services/keepstore/volume.go
+++ b/services/keepstore/volume.go
@@ -57,7 +57,13 @@ type Volume interface {
 	//
 	// If a block is already stored under the same name (loc) with
 	// different content, Put must either overwrite the existing
-	// data with the new data or return a non-nil error.
+	// data with the new data or return a non-nil error. When
+	// overwriting existing data, it must never leave the storage
+	// device in an inconsistent state: a subsequent call to Get
+	// must return either the entire old block, the entire new
+	// block, or an error. (An implementation that cannot peform
+	// atomic updates must leave the old data alone and return an
+	// error.)
 	//
 	// Put also sets the timestamp for the given locator to the
 	// current time.
diff --git a/services/keepstore/volume_generic_test.go b/services/keepstore/volume_generic_test.go
index f1b27be..5f68ef8 100644
--- a/services/keepstore/volume_generic_test.go
+++ b/services/keepstore/volume_generic_test.go
@@ -157,14 +157,24 @@ func testPutBlockWithDifferentContent(t *testing.T, factory TestableVolumeFactor
 		t.Errorf("Got err putting block %q: %q, expected nil", TEST_BLOCK, err)
 	}
 
-	// Whether Put with the same loc with different content fails or succeeds
-	// is implementation dependent. So, just check loc exists after overwriting.
-	// We also do not want to see if loc has block1 or block2, for the same reason.
-	if err = v.Put(TEST_HASH, TEST_BLOCK_2); err != nil {
-		t.Errorf("Got err putting block with different content %q: %q, expected nil", TEST_BLOCK, err)
-	}
-	if _, err := v.Get(TEST_HASH); err != nil {
-		t.Errorf("Got err getting block %q: %q, expected nil", TEST_BLOCK, err)
+	putErr := v.Put(TEST_HASH, TEST_BLOCK_2)
+	buf, getErr := v.Get(TEST_HASH)
+	if putErr == nil {
+		// Put must not return a nil error unless it has
+		// overwritten the existing data.
+		if bytes.Compare(buf, TEST_BLOCK_2) != 0 {
+			t.Errorf("Put succeeded but Get returned %+v, expected %+v", buf, TEST_BLOCK_2)
+		}
+	} else {
+		// It is permissible for Put to fail, but it must
+		// leave us with either the original data, the new
+		// data, or nothing at all.
+		if getErr == nil && bytes.Compare(buf, TEST_BLOCK) != 0 && bytes.Compare(buf, TEST_BLOCK_2) != 0 {
+			t.Errorf("Put failed but Get returned %+v, which is neither %+v nor %+v", buf, TEST_BLOCK, TEST_BLOCK_2)
+		}
+	}
+	if getErr == nil {
+		bufs.Put(buf)
 	}
 }
 

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


hooks/post-receive
--