[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