[ARVADOS] updated: e7d36bd8f4ca443f433c63eac863fec7e61ef121

git at public.curoverse.com git at public.curoverse.com
Tue Oct 13 12:03:42 EDT 2015


Summary of changes:
 services/keepstore/azure_blob_volume.go   | 22 ++++++++++++++++------
 services/keepstore/volume_generic_test.go | 14 ++++++++++++++
 services/keepstore/volume_unix.go         | 17 +++++++++++++++--
 3 files changed, 45 insertions(+), 8 deletions(-)

       via  e7d36bd8f4ca443f433c63eac863fec7e61ef121 (commit)
      from  40c9b26a39c773e806e0a1430774f1787820376f (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 e7d36bd8f4ca443f433c63eac863fec7e61ef121
Author: Tom Clegg <tom at curoverse.com>
Date:   Tue Oct 13 12:11:23 2015 -0400

    7159: Return benign os.ErrNotExist error from Compare to avoid excessive logs. refs #7159

diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index d545aad..657c315 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -155,11 +155,7 @@ func (v *AzureBlobVolume) Get(loc string) ([]byte, error) {
 func (v *AzureBlobVolume) get(loc string) ([]byte, error) {
 	rdr, err := v.bsClient.GetBlob(v.containerName, loc)
 	if err != nil {
-		if strings.Contains(err.Error(), "404 Not Found") {
-			// "storage: service returned without a response body (404 Not Found)"
-			return nil, os.ErrNotExist
-		}
-		return nil, err
+		return nil, v.translateError(err)
 	}
 	defer rdr.Close()
 	buf := bufs.Get(BlockSize)
@@ -176,7 +172,7 @@ func (v *AzureBlobVolume) get(loc string) ([]byte, error) {
 func (v *AzureBlobVolume) Compare(loc string, expect []byte) error {
 	rdr, err := v.bsClient.GetBlob(v.containerName, loc)
 	if err != nil {
-		return err
+		return v.translateError(err)
 	}
 	defer rdr.Close()
 	return compareReaderWithBuf(rdr, expect, loc[:32])
@@ -279,3 +275,17 @@ func (v *AzureBlobVolume) Writable() bool {
 func (v *AzureBlobVolume) Replication() int {
 	return v.replication
 }
+
+// If possible, translate an Azure SDK error to a recognizable error
+// like os.ErrNotExist.
+func (v *AzureBlobVolume) translateError(err error) error {
+	switch {
+	case err == nil:
+		return err
+	case strings.Contains(err.Error(), "404 Not Found"):
+		// "storage: service returned without a response body (404 Not Found)"
+		return os.ErrNotExist
+	default:
+		return err
+	}
+}
diff --git a/services/keepstore/volume_generic_test.go b/services/keepstore/volume_generic_test.go
index 74c5b2a..7ef079f 100644
--- a/services/keepstore/volume_generic_test.go
+++ b/services/keepstore/volume_generic_test.go
@@ -24,6 +24,7 @@ func DoGenericVolumeTests(t *testing.T, factory TestableVolumeFactory) {
 	testGet(t, factory)
 	testGetNoSuchBlock(t, factory)
 
+	testCompareNonexistent(t, factory)
 	testCompareSameContent(t, factory, TestHash, TestBlock)
 	testCompareSameContent(t, factory, EmptyHash, EmptyBlock)
 	testCompareWithCollision(t, factory, TestHash, TestBlock, []byte("baddata"))
@@ -95,6 +96,19 @@ func testGetNoSuchBlock(t *testing.T, factory TestableVolumeFactory) {
 	}
 }
 
+// Compare() should return os.ErrNotExist if the block does not exist.
+// Otherwise, writing new data causes CompareAndTouch() to generate
+// error logs even though everything is working fine.
+func testCompareNonexistent(t *testing.T, factory TestableVolumeFactory) {
+	v := factory(t)
+	defer v.Teardown()
+
+	err := v.Compare(TestHash, TestBlock)
+	if err != os.ErrNotExist {
+		t.Errorf("Got err %T %q, expected os.ErrNotExist", err, err)
+	}
+}
+
 // Put a test block and compare the locator with same content
 // Test should pass for both writable and read-only volumes
 func testCompareSameContent(t *testing.T, factory TestableVolumeFactory, testHash string, testData []byte) {
diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index 98c31d1..e745eb2 100644
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -189,7 +189,7 @@ func (v *UnixVolume) Get(loc string) ([]byte, error) {
 	path := v.blockPath(loc)
 	stat, err := v.stat(path)
 	if err != nil {
-		return nil, err
+		return nil, v.translateError(err)
 	}
 	buf := bufs.Get(int(stat.Size()))
 	err = v.getFunc(path, func(rdr io.Reader) error {
@@ -209,7 +209,7 @@ func (v *UnixVolume) Get(loc string) ([]byte, error) {
 func (v *UnixVolume) Compare(loc string, expect []byte) error {
 	path := v.blockPath(loc)
 	if _, err := v.stat(path); err != nil {
-		return err
+		return v.translateError(err)
 	}
 	return v.getFunc(path, func(rdr io.Reader) error {
 		return compareReaderWithBuf(rdr, expect, loc[:32])
@@ -479,3 +479,16 @@ func lockfile(f *os.File) error {
 func unlockfile(f *os.File) error {
 	return syscall.Flock(int(f.Fd()), syscall.LOCK_UN)
 }
+
+// Where appropriate, translate a more specific filesystem error to an
+// error recognized by handlers, like os.ErrNotExist.
+func (v *UnixVolume) translateError(err error) error {
+	switch err.(type) {
+	case *os.PathError:
+		// stat() returns a PathError if the parent directory
+		// (not just the file itself) is missing
+		return os.ErrNotExist
+	default:
+		return err
+	}
+}

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list