[ARVADOS] created: 70dd308c6d20951745d9b8e849131d0b14e7386a
git at public.curoverse.com
git at public.curoverse.com
Tue Jun 3 16:57:38 EDT 2014
at 70dd308c6d20951745d9b8e849131d0b14e7386a (commit)
commit 70dd308c6d20951745d9b8e849131d0b14e7386a
Author: Tim Pierce <twp at curoverse.com>
Date: Tue Jun 3 16:53:50 2014 -0400
2865: reduce Keep memory usage.
Eliminate ioutil.ReadAll to reduce unnecessary 2x memory allocations.
* PutBlockHandler allocates a buffer exactly as long as
req.ContentLength and fills it with io.ReadFull.
* GetBlock uses ioutil.ReadFile (which it arguably should have been
doing in the first place).
Refs #2865.
diff --git a/services/keep/src/keep/keep.go b/services/keep/src/keep/keep.go
index fa7340d..429a7e0 100644
--- a/services/keep/src/keep/keep.go
+++ b/services/keep/src/keep/keep.go
@@ -5,7 +5,6 @@ import (
"bytes"
"crypto/md5"
"encoding/json"
- "errors"
"flag"
"fmt"
"github.com/gorilla/mux"
@@ -87,10 +86,6 @@ func (e *KeepError) Error() string {
return e.ErrMsg
}
-// This error is returned by ReadAtMost if the available
-// data exceeds BLOCKSIZE bytes.
-var ReadErrorTooLong = errors.New("Too long")
-
// TODO(twp): continue moving as much code as possible out of main
// so it can be effectively tested. Esp. handling and postprocessing
// of command line flags (identifying Keep volumes and initializing
@@ -444,14 +439,18 @@ func PutBlockHandler(resp http.ResponseWriter, req *http.Request) {
// Read the block data to be stored.
// If the request exceeds BLOCKSIZE bytes, issue a HTTP 500 error.
//
- // Note: because req.Body is a buffered Reader, each Read() call will
- // collect only the data in the network buffer (typically 16384 bytes),
- // even if it is passed a much larger slice.
- //
- // Instead, call ReadAtMost to read data from the socket
- // repeatedly until either EOF or BLOCKSIZE bytes have been read.
- //
- if buf, err := ReadAtMost(req.Body, BLOCKSIZE); err == nil {
+ if req.ContentLength > BLOCKSIZE {
+ http.Error(resp, TooLongError.Error(), TooLongError.HTTPCode)
+ return
+ }
+
+ buf := make([]byte, req.ContentLength)
+ nread, err := io.ReadFull(req.Body, buf)
+ if err != nil {
+ http.Error(resp, err.Error(), 500)
+ } else if int64(nread) < req.ContentLength {
+ http.Error(resp, "request truncated", 500)
+ } else {
if err := PutBlock(buf, hash); err == nil {
// Success; add a size hint, sign the locator if
// possible, and return it to the client.
@@ -466,16 +465,8 @@ func PutBlockHandler(resp http.ResponseWriter, req *http.Request) {
ke := err.(*KeepError)
http.Error(resp, ke.Error(), ke.HTTPCode)
}
- } else {
- log.Println("error reading request: ", err)
- errmsg := err.Error()
- if err == ReadErrorTooLong {
- // Use a more descriptive error message that includes
- // the maximum request size.
- errmsg = fmt.Sprintf("Max request size %d bytes", BLOCKSIZE)
- }
- http.Error(resp, errmsg, 500)
}
+ return
}
// IndexHandler
@@ -691,24 +682,6 @@ func PutBlock(block []byte, hash string) error {
}
}
-// ReadAtMost
-// Reads bytes repeatedly from an io.Reader until either
-// encountering EOF, or the maxbytes byte limit has been reached.
-// Returns a byte slice of the bytes that were read.
-//
-// If the reader contains more than maxbytes, returns a nil slice
-// and an error.
-//
-func ReadAtMost(r io.Reader, maxbytes int) ([]byte, error) {
- // Attempt to read one more byte than maxbytes.
- lr := io.LimitReader(r, int64(maxbytes+1))
- buf, err := ioutil.ReadAll(lr)
- if len(buf) > maxbytes {
- return nil, ReadErrorTooLong
- }
- return buf, err
-}
-
// IsValidLocator
// Return true if the specified string is a valid Keep locator.
// When Keep is extended to support hash types other than MD5,
diff --git a/services/keep/src/keep/volume_unix.go b/services/keep/src/keep/volume_unix.go
index 88410de..7b711d2 100644
--- a/services/keep/src/keep/volume_unix.go
+++ b/services/keep/src/keep/volume_unix.go
@@ -109,23 +109,13 @@ func (v *UnixVolume) Put(loc string, block []byte) error {
// corrupted data block.
//
func (v *UnixVolume) Read(loc string) ([]byte, error) {
- var f *os.File
- var err error
- var buf []byte
-
blockFilename := filepath.Join(v.root, loc[0:3], loc)
-
- f, err = os.Open(blockFilename)
+ buf, err := ioutil.ReadFile(blockFilename)
if err != nil {
- return nil, err
- }
-
- if buf, err = ioutil.ReadAll(f); err != nil {
log.Printf("%s: reading %s: %s\n", v, blockFilename, err)
- return buf, err
+ return nil, err
}
- // Success!
return buf, nil
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list