[ARVADOS] updated: 852325b0c1523a37d780e7e5e0a76c80733eea66

git at public.curoverse.com git at public.curoverse.com
Fri May 8 14:06:26 EDT 2015


Summary of changes:
 services/crunchstat/crunchstat.go | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

  discards  5acaf1e58df8ee85a8026b82be72966f8752d740 (commit)
  discards  2ddfdc072e53b581d1aae418136f1a5f8f495abf (commit)
  discards  10c7a157c03405a6986fdd8924d7f45a71803875 (commit)
  discards  8522f4fd87725ade1f6d9878f50b85530b383baf (commit)
  discards  1003636e14221bfd43cf29c1c81e1e6bc31f7445 (commit)
       via  852325b0c1523a37d780e7e5e0a76c80733eea66 (commit)
       via  a7af112f0554002ec1ce07312c39e480fc7437f7 (commit)
       via  4e26efd8ee2d1896e329dc32488346341557a15b (commit)
       via  b25f82e548caf7f731f7c4c506b757a6a1a531f0 (commit)
       via  14a27997ba2a94c5a7250ddde4519d5f68b6eda0 (commit)
       via  31d8ece649c63b008ed79a930a5b237ec795ff22 (commit)
       via  81d100b157dee4f24d376cdd8b5d739b385c925e (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (5acaf1e58df8ee85a8026b82be72966f8752d740)
            \
             N -- N -- N (852325b0c1523a37d780e7e5e0a76c80733eea66)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

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 852325b0c1523a37d780e7e5e0a76c80733eea66
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri May 8 14:08:19 2015 -0400

    5748: Update keepstore install docs (-max-buffers, -blob-signing-key).

diff --git a/doc/install/install-keepstore.html.textile.liquid b/doc/install/install-keepstore.html.textile.liquid
index 7fb810d..eb53783 100644
--- a/doc/install/install-keepstore.html.textile.liquid
+++ b/doc/install/install-keepstore.html.textile.liquid
@@ -33,31 +33,36 @@ Verify that Keepstore is functional:
 
 <notextile>
 <pre><code>~$ <span class="userinput">keepstore -h</span>
-2014/10/29 14:23:38 Keep started: pid 6848
-Usage of keepstore:
+2015/05/08 13:41:16 keepstore starting, pid 2565
+Usage of ./keepstore:
+  -blob-signature-ttl=1209600: Lifetime of blob permission signatures. See services/api/config/application.default.yml.
+  -blob-signing-key-file="": File containing the secret key for generating and verifying blob permission signatures.
   -data-manager-token-file="": File with the API token used by the Data Manager. All DELETE requests or GET /index requests must carry this token.
   -enforce-permissions=false: Enforce permission signatures on requests.
-  -listen=":25107": Interface on which to listen for requests, in the format ipaddr:port. e.g. -listen=10.0.1.24:8000. Use -listen=:port to listen on all network interfaces.
+  -listen=":25107": Listening address, in the form "host:port". e.g., 10.0.1.24:8000. Omit the host part to listen on all interfaces.
+  -max-buffers=128: Maximum RAM to use for data buffers, given in multiples of block size (64 MiB). When this limit is reached, HTTP requests requiring buffers (like GET and PUT) will wait for buffer space to be released.
   -never-delete=false: If set, nothing will be deleted. HTTP 405 will be returned for valid DELETE requests.
-  -permission-key-file="": File containing the secret key for generating and verifying permission signatures.
-  -permission-ttl=1209600: Expiration time (in seconds) for newly generated permission signatures.
-  -pid="": Path to write pid file
-  -serialize=false: If set, all read and write operations on local Keep volumes will be serialized.
-  -volumes="": Comma-separated list of directories to use for Keep volumes, e.g. -volumes=/var/keep1,/var/keep2. If empty or not supplied, Keep will scan mounted filesystems for volumes with a /keep top-level directory.
+  -permission-key-file="": Synonym for -blob-signing-key-file.
+  -permission-ttl=0: Synonym for -blob-signature-ttl.
+  -pid="": Path to write pid file during startup. This file is kept open and locked with LOCK_EX until keepstore exits, so `fuser -k pidfile` is one way to shut down. Exit immediately if there is an error opening, locking, or writing the pid file.
+  -readonly=false: Do not write, delete, or touch anything on the following volumes.
+  -serialize=false: Serialize read and write operations on the following volumes.
+  -volume=[]: Local storage directory. Can be given more than once to add multiple directories. If none are supplied, the default is to use all directories named "keep" that exist in the top level directory of a mount point at startup time. Can be a comma-separated list, but this is deprecated: use multiple -volume arguments instead.
+  -volumes=[]: Deprecated synonym for -volume.
 </code></pre>
 </notextile>
 
-If you want access control on your Keepstore server(s), you should provide a permission key. The @-permission-key-file@ argument should contain the path to a file that contains a single line with a long random alphanumeric string. It should be the same as the @blob_signing_key@ that can be set in the "API server":install-api-server.html config/application.yml file.
+If you want access control on your Keepstore server(s), you must specify the @-enforce-permissions@ flag and provide a signing key. The @-blob-signing-key-file@ argument should be a file containing a long random alphanumeric string with no internal line breaks (it is also possible to use a socket or FIFO: keepstore reads it only once, at startup). This key must be the same as the @blob_signing_key@ configured in the "API server":install-api-server.html config/application.yml file.
+
+The @-max-buffers@ argument can be used to restrict keepstore's memory use. By default, keepstore will allocate no more than 128 blocks (8 GiB) worth of data buffers at a time. Normally this should be set as high as possible without risking swapping.
 
 Prepare one or more volumes for Keepstore to use. Simply create a /keep directory on all the partitions you would like Keepstore to use, and then start Keepstore. For example, using 2 tmpfs volumes:
 
 <notextile>
-<pre><code>~$ <span class="userinput">keepstore</span>
-2014/10/29 11:41:37 Keep started: pid 20736
-2014/10/29 11:41:37 adding Keep volume: /tmp/tmp.vwSCtUCyeH/keep
-2014/10/29 11:41:37 adding Keep volume: /tmp/tmp.Lsn4w8N3Xv/keep
-2014/10/29 11:41:37 Running without a PermissionSecret. Block locators returned by this server will not be signed, and will be rejected by a server that enforces permissions.
-2014/10/29 11:41:37 To fix this, run Keep with --permission-key-file=<path> to define the location of a file containing the permission key.
+<pre><code>~$ <span class="userinput">keepstore -blob-signing-key-file=./blob-signing-key</span>
+2015/05/08 13:44:26 keepstore starting, pid 2765
+2015/05/08 13:44:26 Using volume [UnixVolume /mnt/keep] (writable=true)
+2015/05/08 13:44:26 listening at :25107
 
 </code></pre>
 </notextile>
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index 175897c..06b2f6f 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -355,7 +355,7 @@ func main() {
 			log.Println("Running without a PermissionSecret. Block locators " +
 				"returned by this server will not be signed, and will be rejected " +
 				"by a server that enforces permissions.")
-			log.Println("To fix this, use the -permission-key-file flag " +
+			log.Println("To fix this, use the -blob-signing-key-file flag " +
 				"to specify the file containing the permission key.")
 		}
 	}

commit a7af112f0554002ec1ce07312c39e480fc7437f7
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri May 8 01:17:40 2015 -0400

    5748: Log clock time for each request.
    
    For successful responses, log the standard HTTP status text for the
    response code (instead of logging "OK" for any status<400).

diff --git a/services/keepstore/logging_router.go b/services/keepstore/logging_router.go
index e30df87..b622d1d 100644
--- a/services/keepstore/logging_router.go
+++ b/services/keepstore/logging_router.go
@@ -8,6 +8,7 @@ import (
 	"log"
 	"net/http"
 	"strings"
+	"time"
 )
 
 type LoggingResponseWriter struct {
@@ -40,12 +41,13 @@ func MakeLoggingRESTRouter() *LoggingRESTRouter {
 }
 
 func (loggingRouter *LoggingRESTRouter) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
-	loggingWriter := LoggingResponseWriter{200, 0, resp, ""}
+	t0 := time.Now()
+	loggingWriter := LoggingResponseWriter{http.StatusOK, 0, resp, ""}
 	loggingRouter.router.ServeHTTP(&loggingWriter, req)
-	statusText := "OK"
+	statusText := http.StatusText(loggingWriter.Status)
 	if loggingWriter.Status >= 400 {
 		statusText = strings.Replace(loggingWriter.ResponseBody, "\n", "", -1)
 	}
-	log.Printf("[%s] %s %s %d %d \"%s\"", req.RemoteAddr, req.Method, req.URL.Path[1:], loggingWriter.Status, loggingWriter.Length, statusText)
+	log.Printf("[%s] %s %s %.6fs %d %d \"%s\"", req.RemoteAddr, req.Method, req.URL.Path[1:], time.Since(t0).Seconds(), loggingWriter.Status, loggingWriter.Length, statusText)
 
 }

commit 4e26efd8ee2d1896e329dc32488346341557a15b
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu May 7 17:55:12 2015 -0400

    5748: Check for errors when writing pidfile. Keep it locked while running.
    
    * Handle SIGINT (the same way as SIGTERM)
    
    * Improve startup/shutdown logs

diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index c0b8aec..175897c 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -202,7 +202,8 @@ func (vs *volumeSet) Discover() int {
 // permission arguments).
 
 func main() {
-	log.Println("Keep started: pid", os.Getpid())
+	log.Println("keepstore starting, pid", os.Getpid())
+	defer log.Println("keepstore exiting, pid", os.Getpid())
 
 	var (
 		data_manager_token_file string
@@ -278,7 +279,7 @@ func main() {
 		&pidfile,
 		"pid",
 		"",
-		"Path to write pid file")
+		"Path to write pid file during startup. This file is kept open and locked with LOCK_EX until keepstore exits, so `fuser -k pidfile` is one way to shut down. Exit immediately if there is an error opening, locking, or writing the pid file.")
 	flag.IntVar(
 		&maxBuffers,
 		"max-buffers",
@@ -292,6 +293,31 @@ func main() {
 	}
 	bufs = newBufferPool(maxBuffers, BLOCKSIZE)
 
+	if pidfile != "" {
+		f, err := os.OpenFile(pidfile, os.O_RDWR | os.O_CREATE, 0777)
+		if err != nil {
+			log.Fatalf("open pidfile (%s): %s", pidfile, err)
+		}
+		err = syscall.Flock(int(f.Fd()), syscall.LOCK_EX | syscall.LOCK_NB)
+		if err != nil {
+			log.Fatalf("flock pidfile (%s): %s", pidfile, err)
+		}
+		err = f.Truncate(0)
+		if err != nil {
+			log.Fatalf("truncate pidfile (%s): %s", pidfile, err)
+		}
+		_, err = fmt.Fprint(f, os.Getpid())
+		if err != nil {
+			log.Fatalf("write pidfile (%s): %s", pidfile, err)
+		}
+		err = f.Sync()
+		if err != nil {
+			log.Fatalf("sync pidfile (%s): %s", pidfile, err)
+		}
+		defer f.Close()
+		defer os.Remove(pidfile)
+	}
+
 	if len(volumes) == 0 {
 		if volumes.Discover() == 0 {
 			log.Fatal("No volumes found.")
@@ -374,24 +400,9 @@ func main() {
 		listener.Close()
 	}(term)
 	signal.Notify(term, syscall.SIGTERM)
+	signal.Notify(term, syscall.SIGINT)
 
-	if pidfile != "" {
-		f, err := os.Create(pidfile)
-		if err == nil {
-			fmt.Fprint(f, os.Getpid())
-			f.Close()
-		} else {
-			log.Printf("Error writing pid file (%s): %s", pidfile, err.Error())
-		}
-	}
-
-	// Start listening for requests.
+	log.Println("listening at", listen)
 	srv := &http.Server{Addr: listen}
 	srv.Serve(listener)
-
-	log.Println("shutting down")
-
-	if pidfile != "" {
-		os.Remove(pidfile)
-	}
 }

commit b25f82e548caf7f731f7c4c506b757a6a1a531f0
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu May 7 12:38:12 2015 -0400

    5748: Return the real decoder error for unparseable trash and pull requests.

diff --git a/services/keepstore/handler_test.go b/services/keepstore/handler_test.go
index 5b65cd8..6823ad0 100644
--- a/services/keepstore/handler_test.go
+++ b/services/keepstore/handler_test.go
@@ -636,7 +636,7 @@ func TestPullHandler(t *testing.T) {
 			"Invalid pull request from the data manager",
 			RequestTester{"/pull", data_manager_token, "PUT", bad_json},
 			http.StatusBadRequest,
-			"Bad Request\n",
+			"",
 		},
 	}
 
@@ -740,7 +740,7 @@ func TestTrashHandler(t *testing.T) {
 			"Invalid trash list from the data manager",
 			RequestTester{"/trash", data_manager_token, "PUT", bad_json},
 			http.StatusBadRequest,
-			"Bad Request\n",
+			"",
 		},
 	}
 
@@ -798,7 +798,7 @@ func ExpectBody(
 	testname string,
 	expected_body string,
 	response *httptest.ResponseRecorder) {
-	if response.Body.String() != expected_body {
+	if expected_body != "" && response.Body.String() != expected_body {
 		t.Errorf("%s: expected response body '%s', got %+v",
 			testname, expected_body, response)
 	}
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index e0e3c61..cf5dfca 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -409,7 +409,7 @@ func PullHandler(resp http.ResponseWriter, req *http.Request) {
 	var pr []PullRequest
 	r := json.NewDecoder(req.Body)
 	if err := r.Decode(&pr); err != nil {
-		http.Error(resp, BadRequestError.Error(), BadRequestError.HTTPCode)
+		http.Error(resp, err.Error(), BadRequestError.HTTPCode)
 		return
 	}
 
@@ -443,7 +443,7 @@ func TrashHandler(resp http.ResponseWriter, req *http.Request) {
 	var trash []TrashRequest
 	r := json.NewDecoder(req.Body)
 	if err := r.Decode(&trash); err != nil {
-		http.Error(resp, BadRequestError.Error(), BadRequestError.HTTPCode)
+		http.Error(resp, err.Error(), BadRequestError.HTTPCode)
 		return
 	}
 

commit 14a27997ba2a94c5a7250ddde4519d5f68b6eda0
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu May 7 10:40:44 2015 -0400

    5748: Use a buffer pool instead of calling runtime.GC() during each GET.

diff --git a/services/crunchstat/.gitignore b/services/crunchstat/.gitignore
new file mode 100644
index 0000000..c26270a
--- /dev/null
+++ b/services/crunchstat/.gitignore
@@ -0,0 +1 @@
+crunchstat
diff --git a/services/keepproxy/.gitignore b/services/keepproxy/.gitignore
new file mode 100644
index 0000000..a4c8ad9
--- /dev/null
+++ b/services/keepproxy/.gitignore
@@ -0,0 +1 @@
+keepproxy
diff --git a/services/keepstore/.gitignore b/services/keepstore/.gitignore
new file mode 100644
index 0000000..c195c4a
--- /dev/null
+++ b/services/keepstore/.gitignore
@@ -0,0 +1 @@
+keepstore
diff --git a/services/keepstore/bufferpool.go b/services/keepstore/bufferpool.go
new file mode 100644
index 0000000..373bfc7
--- /dev/null
+++ b/services/keepstore/bufferpool.go
@@ -0,0 +1,44 @@
+package main
+
+import (
+	"log"
+	"sync"
+	"time"
+)
+
+type bufferPool struct {
+	// limiter has a "true" placeholder for each in-use buffer.
+	limiter chan bool
+	// Pool has unused buffers.
+	sync.Pool
+}
+
+func newBufferPool(count int, bufSize int) *bufferPool {
+	p := bufferPool{}
+	p.New = func() interface{} {
+		return make([]byte, bufSize)
+	}
+	p.limiter = make(chan bool, count)
+	return &p
+}
+
+func (p *bufferPool) Get(size int) []byte {
+	select {
+	case p.limiter <- true:
+	default:
+		t0 := time.Now()
+		log.Printf("reached max buffers (%d), waiting", cap(p.limiter))
+		p.limiter <- true
+		log.Printf("waited %v for a buffer", time.Since(t0))
+	}
+	buf := p.Pool.Get().([]byte)
+	if cap(buf) < size {
+		log.Fatalf("bufferPool Get(size=%d) but max=%d", size, cap(buf))
+	}
+	return buf[:size]
+}
+
+func (p *bufferPool) Put(buf []byte) {
+	p.Pool.Put(buf)
+	<-p.limiter
+}
diff --git a/services/keepstore/bufferpool_test.go b/services/keepstore/bufferpool_test.go
new file mode 100644
index 0000000..b2f63b1
--- /dev/null
+++ b/services/keepstore/bufferpool_test.go
@@ -0,0 +1,85 @@
+package main
+
+import (
+	. "gopkg.in/check.v1"
+	"testing"
+	"time"
+)
+
+// Gocheck boilerplate
+func TestBufferPool(t *testing.T) {
+	TestingT(t)
+}
+var _ = Suite(&BufferPoolSuite{})
+type BufferPoolSuite struct {}
+
+// Initialize a default-sized buffer pool for the benefit of test
+// suites that don't run main().
+func init() {
+	bufs = newBufferPool(maxBuffers, BLOCKSIZE)
+}
+
+func (s *BufferPoolSuite) TestBufferPoolBufSize(c *C) {
+	bufs := newBufferPool(2, 10)
+	b1 := bufs.Get(1)
+	bufs.Get(2)
+	bufs.Put(b1)
+	b3 := bufs.Get(3)
+	c.Check(len(b3), Equals, 3)
+}
+
+func (s *BufferPoolSuite) TestBufferPoolUnderLimit(c *C) {
+	bufs := newBufferPool(3, 10)
+	b1 := bufs.Get(10)
+	bufs.Get(10)
+	testBufferPoolRace(c, bufs, b1, "Get")
+}
+
+func (s *BufferPoolSuite) TestBufferPoolAtLimit(c *C) {
+	bufs := newBufferPool(2, 10)
+	b1 := bufs.Get(10)
+	bufs.Get(10)
+	testBufferPoolRace(c, bufs, b1, "Put")
+}
+
+func testBufferPoolRace(c *C, bufs *bufferPool, unused []byte, expectWin string) {
+	race := make(chan string)
+	go func() {
+		bufs.Get(10)
+		time.Sleep(time.Millisecond)
+		race <- "Get"
+	}()
+	go func() {
+		time.Sleep(10*time.Millisecond)
+		bufs.Put(unused)
+		race <- "Put"
+	}()
+	c.Check(<-race, Equals, expectWin)
+	c.Check(<-race, Not(Equals), expectWin)
+	close(race)
+}
+
+func (s *BufferPoolSuite) TestBufferPoolReuse(c *C) {
+	bufs := newBufferPool(2, 10)
+	bufs.Get(10)
+	last := bufs.Get(10)
+	// The buffer pool is allowed to throw away unused buffers
+	// (e.g., during sync.Pool's garbage collection hook, in the
+	// the current implementation). However, if unused buffers are
+	// getting thrown away and reallocated more than {arbitrary
+	// frequency threshold} during a busy loop, it's not acting
+	// much like a buffer pool.
+	allocs := 1000
+	reuses := 0
+	for i := 0; i < allocs; i++ {
+		bufs.Put(last)
+		next := bufs.Get(10)
+		copy(last, []byte("last"))
+		copy(next, []byte("next"))
+		if last[0] == 'n' {
+			reuses++
+		}
+		last = next
+	}
+	c.Check(reuses > allocs * 95/100, Equals, true)
+}
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index 75b56eb..e0e3c61 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -112,25 +112,17 @@ func GetBlockHandler(resp http.ResponseWriter, req *http.Request) {
 	}
 
 	block, err := GetBlock(hash, false)
-
-	// Garbage collect after each GET. Fixes #2865.
-	// TODO(twp): review Keep memory usage and see if there's
-	// a better way to do this than blindly garbage collecting
-	// after every block.
-	defer runtime.GC()
-
 	if err != nil {
 		// This type assertion is safe because the only errors
 		// GetBlock can return are DiskHashError or NotFoundError.
 		http.Error(resp, err.Error(), err.(*KeepError).HTTPCode)
 		return
 	}
+	defer bufs.Put(block)
 
-	resp.Header().Set("Content-Length", fmt.Sprintf("%d", len(block)))
-
-	_, err = resp.Write(block)
-
-	return
+	resp.Header().Set("Content-Length", strconv.Itoa(len(block)))
+	resp.Header().Set("Content-Type", "application/octet-stream")
+	resp.Write(block)
 }
 
 func PutBlockHandler(resp http.ResponseWriter, req *http.Request) {
@@ -159,17 +151,17 @@ func PutBlockHandler(resp http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	buf := make([]byte, req.ContentLength)
-	nread, err := io.ReadFull(req.Body, buf)
+	buf := bufs.Get(int(req.ContentLength))
+	_, err := io.ReadFull(req.Body, buf)
 	if err != nil {
 		http.Error(resp, err.Error(), 500)
-		return
-	} else if int64(nread) < req.ContentLength {
-		http.Error(resp, "request truncated", 500)
+		bufs.Put(buf)
 		return
 	}
 
 	err = PutBlock(buf, hash)
+	bufs.Put(buf)
+
 	if err != nil {
 		ke := err.(*KeepError)
 		http.Error(resp, ke.Error(), ke.HTTPCode)
@@ -178,7 +170,7 @@ func PutBlockHandler(resp http.ResponseWriter, req *http.Request) {
 
 	// Success; add a size hint, sign the locator if possible, and
 	// return it to the client.
-	return_hash := fmt.Sprintf("%s+%d", hash, len(buf))
+	return_hash := fmt.Sprintf("%s+%d", hash, req.ContentLength)
 	api_token := GetApiToken(req)
 	if PermissionSecret != nil && api_token != "" {
 		expiry := time.Now().Add(blob_signature_ttl)
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index 71e577f..c0b8aec 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -56,6 +56,9 @@ var data_manager_token string
 // actually deleting anything.
 var never_delete = false
 
+var maxBuffers = 128
+var bufs *bufferPool
+
 // ==========
 // Error types.
 //
@@ -276,9 +279,19 @@ func main() {
 		"pid",
 		"",
 		"Path to write pid file")
+	flag.IntVar(
+		&maxBuffers,
+		"max-buffers",
+		maxBuffers,
+		fmt.Sprintf("Maximum RAM to use for data buffers, given in multiples of block size (%d MiB). When this limit is reached, HTTP requests requiring buffers (like GET and PUT) will wait for buffer space to be released.", BLOCKSIZE>>20))
 
 	flag.Parse()
 
+	if maxBuffers < 0 {
+		log.Fatal("-max-buffers must be greater than zero.")
+	}
+	bufs = newBufferPool(maxBuffers, BLOCKSIZE)
+
 	if len(volumes) == 0 {
 		if volumes.Discover() == 0 {
 			log.Fatal("No volumes found.")
diff --git a/services/keepstore/volume.go b/services/keepstore/volume.go
index 1b52949..64fea34 100644
--- a/services/keepstore/volume.go
+++ b/services/keepstore/volume.go
@@ -11,6 +11,9 @@ import (
 )
 
 type Volume interface {
+	// Get a block. IFF the returned error is nil, the caller must
+	// put the returned slice back into the buffer pool when it's
+	// finished with it.
 	Get(loc string) ([]byte, error)
 	Put(loc string, block []byte) error
 	Touch(loc string) error
diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go
index 66e0810..2615019 100644
--- a/services/keepstore/volume_test.go
+++ b/services/keepstore/volume_test.go
@@ -65,7 +65,9 @@ func (v *MockVolume) Get(loc string) ([]byte, error) {
 	if v.Bad {
 		return nil, errors.New("Bad volume")
 	} else if block, ok := v.Store[loc]; ok {
-		return block, nil
+		buf := bufs.Get(len(block))
+		copy(buf, block)
+		return buf, nil
 	}
 	return nil, os.ErrNotExist
 }
diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index bcf57c1..61a98b5 100644
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -63,15 +63,33 @@ func (v *UnixVolume) Mtime(loc string) (time.Time, error) {
 // slice and whatever non-nil error was returned by Stat or ReadFile.
 func (v *UnixVolume) Get(loc string) ([]byte, error) {
 	path := v.blockPath(loc)
-	if _, err := os.Stat(path); err != nil {
+	stat, err := os.Stat(path)
+	if err != nil {
+		return nil, err
+	}
+	if stat.Size() < 0 {
+		return nil, os.ErrInvalid
+	} else if stat.Size() == 0 {
+		return bufs.Get(0), nil
+	} else if stat.Size() > BLOCKSIZE {
+		return nil, TooLongError
+	}
+	f, err := os.Open(path)
+	if err != nil {
 		return nil, err
 	}
+	defer f.Close()
+	buf := bufs.Get(int(stat.Size()))
 	if v.serialize {
 		v.mutex.Lock()
 		defer v.mutex.Unlock()
 	}
-	buf, err := ioutil.ReadFile(path)
-	return buf, err
+	_, err = io.ReadFull(f, buf)
+	if err != nil {
+		bufs.Put(buf)
+		return nil, err
+	}
+	return buf, nil
 }
 
 // Put stores a block of data identified by the locator string

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list