[ARVADOS] updated: b452636351b19ed5641a20eabc68881bd8905ca4
git at public.curoverse.com
git at public.curoverse.com
Thu Apr 10 16:32:52 EDT 2014
Summary of changes:
sdk/cli/bin/crunch-job | 1 +
services/api/app/models/user.rb | 14 ++++---
services/api/config/database.yml.sample | 4 +-
services/api/test/fixtures/jobs.yml | 2 +
services/api/test/unit/user_test.rb | 10 ++++-
services/keep/keep.go | 70 +++++++++++++------------------
services/keep/keep_test.go | 48 +++++++++++++++------
7 files changed, 84 insertions(+), 65 deletions(-)
via b452636351b19ed5641a20eabc68881bd8905ca4 (commit)
via bb3a7e08a90ea7826c0b0e1289c64b395b472151 (commit)
via e35ed29187d83ebd4cbc493b9251119013825ac1 (commit)
via 71b0d0fb51e3c54a7959f51fd4dbf523fbaf57db (commit)
via 6ea2e62b70a015226c4f3361ab3591509100a820 (commit)
via e4b0ff638bb41ce55ab3770c4f2b7f744d653aac (commit)
via 50ee9817061629f9ffe2568f937149f6e877df04 (commit)
from a6c0c9dccbe036132c110817d69c08b757aab5fa (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 b452636351b19ed5641a20eabc68881bd8905ca4
Merge: bb3a7e0 e35ed29
Author: Tim Pierce <twp at curoverse.com>
Date: Thu Apr 10 16:34:45 2014 -0400
Merge branch 'master' into 2449-keep-write-blocks
commit bb3a7e08a90ea7826c0b0e1289c64b395b472151
Author: Tim Pierce <twp at curoverse.com>
Date: Thu Apr 10 16:31:47 2014 -0400
Update for code review (refs #2292).
* Use ioutil.TempFile to write new blocks, renaming to the correct
name upon success.
* PutBlock should overwrite corrupt blocks, not report a collision.
* Get free disk space from syscall.Statfs, not `df`.
diff --git a/services/keep/keep.go b/services/keep/keep.go
index 5083e69..7cca534 100644
--- a/services/keep/keep.go
+++ b/services/keep/keep.go
@@ -7,12 +7,13 @@ import (
"errors"
"fmt"
"github.com/gorilla/mux"
+ "io/ioutil"
"log"
"net/http"
"os"
- "os/exec"
"strconv"
"strings"
+ "syscall"
"time"
)
@@ -228,18 +229,16 @@ func PutBlock(block []byte, hash string) error {
}
// If we already have a block on disk under this identifier, return
- // success (but check for MD5 collisions, which may signify on-disk corruption).
+ // success (but check for MD5 collisions).
+ // The only errors that GetBlock can return are ErrCorrupt and ErrNotFound.
+ // In either case, we want to write our new (good) block to disk, so there is
+ // nothing special to do if err != nil.
if oldblock, err := GetBlock(hash); err == nil {
if bytes.Compare(block, oldblock) == 0 {
return nil
} else {
return &KeepError{ErrCollision, errors.New("Collision")}
}
- } else {
- ke := err.(*KeepError)
- if ke.HTTPCode == ErrCorrupt {
- return &KeepError{ErrCollision, errors.New("Collision")}
- }
}
// Store the block on the first available Keep volume.
@@ -256,21 +255,28 @@ func PutBlock(block []byte, hash string) error {
continue
}
- blockFilename := fmt.Sprintf("%s/%s", blockDir, hash)
-
- f, err := os.OpenFile(blockFilename, os.O_CREATE|os.O_WRONLY, 0644)
- if err != nil {
- log.Printf("%s: creating %s: %s\n", vol, blockFilename, err)
+ tmpfile, tmperr := ioutil.TempFile(blockDir, "tmp"+hash)
+ if tmperr != nil {
+ log.Printf("ioutil.TempFile(%s, tmp%s): %s", blockDir, hash, tmperr)
continue
}
+ blockFilename := fmt.Sprintf("%s/%s", blockDir, hash)
- if _, err := f.Write(block); err == nil {
- f.Close()
- return nil
- } else {
+ if _, err := tmpfile.Write(block); err != nil {
log.Printf("%s: writing to %s: %s\n", vol, blockFilename, err)
continue
}
+ if err := tmpfile.Close(); err != nil {
+ log.Printf("closing %s: %s\n", tmpfile.Name(), err)
+ os.Remove(tmpfile.Name())
+ continue
+ }
+ if err := os.Rename(tmpfile.Name(), blockFilename); err != nil {
+ log.Printf("rename %s %s: %s\n", tmpfile.Name(), blockFilename, err)
+ os.Remove(tmpfile.Name())
+ continue
+ }
+ return nil
}
if allFull {
@@ -314,32 +320,14 @@ func IsFull(volume string) (isFull bool) {
// Returns the amount of available disk space on VOLUME,
// as a number of 1k blocks.
//
-func FreeDiskSpace(volume string) (free int, err error) {
- // Run df to find out how much disk space is left.
- cmd := exec.Command("df", "--block-size=1k", volume)
- stdout, perr := cmd.StdoutPipe()
- if perr != nil {
- return 0, perr
- }
- scanner := bufio.NewScanner(stdout)
- if perr := cmd.Start(); err != nil {
- return 0, perr
- }
-
- scanner.Scan() // skip header line of df output
- scanner.Scan()
-
- f := strings.Fields(scanner.Text())
- if avail, err := strconv.Atoi(f[3]); err == nil {
- free = avail
- } else {
- err = errors.New("bad df format: " + scanner.Text())
- }
-
- // Flush the df output and shut it down cleanly.
- for scanner.Scan() {
+func FreeDiskSpace(volume string) (free uint64, err error) {
+ var fs syscall.Statfs_t
+ err = syscall.Statfs(volume, &fs)
+ if err == nil {
+ // Statfs output is not guaranteed to measure free
+ // space in terms of 1K blocks.
+ free = fs.Bavail * uint64(fs.Bsize) / 1024
}
- cmd.Wait()
return
}
diff --git a/services/keep/keep_test.go b/services/keep/keep_test.go
index cb5e89c..3f09e90 100644
--- a/services/keep/keep_test.go
+++ b/services/keep/keep_test.go
@@ -1,6 +1,7 @@
package main
import (
+ "bytes"
"fmt"
"io/ioutil"
"os"
@@ -12,6 +13,17 @@ var TEST_BLOCK = []byte("The quick brown fox jumps over the lazy dog.")
var TEST_HASH = "e4d909c290d0fb1ca068ffaddf22cbd0"
var BAD_BLOCK = []byte("The magic words are squeamish ossifrage.")
+// TODO(twp): Tests still to be written
+//
+// * PutBlockCollision
+// - test that PutBlock(BLOCK, HASH) reports a collision. HASH must
+// be present in Keep and identify a block which sums to HASH but
+// which does not match BLOCK. (Requires an interface to mock MD5.)
+//
+// * PutBlockFull
+// - test that PutBlock returns 503 Full if the filesystem is full.
+// (must mock FreeDiskSpace or Statfs? use a tmpfs?)
+
// ========================================
// GetBlock tests.
// ========================================
@@ -49,6 +61,11 @@ func TestGetBlockMissing(t *testing.T) {
result, err := GetBlock(TEST_HASH)
if err == nil {
t.Errorf("GetBlock incorrectly returned success: ", result)
+ } else {
+ ke := err.(*KeepError)
+ if ke.HTTPCode != ErrNotFound {
+ t.Errorf("GetBlock: %v", ke)
+ }
}
}
@@ -157,11 +174,11 @@ func TestPutBlockMD5Fail(t *testing.T) {
}
}
-// TestPutBlockCollision
-// PutBlock must report a 400 Collision error when asked to store a block
-// when a different block exists on disk under the same identifier.
+// TestPutBlockCorrupt
+// PutBlock should overwrite corrupt blocks on disk when given
+// a PUT request with a good block.
//
-func TestPutBlockCollision(t *testing.T) {
+func TestPutBlockCorrupt(t *testing.T) {
defer teardown()
// Create two test Keep volumes.
@@ -169,20 +186,22 @@ func TestPutBlockCollision(t *testing.T) {
// Store a corrupted block under TEST_HASH.
store(t, KeepVolumes[0], TEST_HASH, BAD_BLOCK)
-
- // Attempting to put TEST_BLOCK should produce a 400 Collision error.
- if err := PutBlock(TEST_BLOCK, TEST_HASH); err == nil {
- t.Error("Expected PutBlock error, but no error returned")
- } else {
- ke := err.(*KeepError)
- if ke.HTTPCode != ErrCollision {
- t.Errorf("Expected 400 Collision error, got %v", ke)
- }
+ if err := PutBlock(TEST_BLOCK, TEST_HASH); err != nil {
+ t.Errorf("PutBlock: %v", err)
}
- KeepVolumes = nil
+ // The block on disk should now match TEST_BLOCK.
+ if block, err := GetBlock(TEST_HASH); err != nil {
+ t.Errorf("GetBlock: %v", err)
+ } else if bytes.Compare(block, TEST_BLOCK) != 0 {
+ t.Errorf("GetBlock returned: '%s'", string(block))
+ }
}
+// ========================================
+// FindKeepVolumes tests.
+// ========================================
+
// TestFindKeepVolumes
// Confirms that FindKeepVolumes finds tmpfs volumes with "/keep"
// directories at the top level.
@@ -272,6 +291,7 @@ func teardown() {
for _, vol := range KeepVolumes {
os.RemoveAll(path.Dir(vol))
}
+ KeepVolumes = nil
}
// store
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list