[ARVADOS] updated: e7a2db3fe3446929a481f20e1358337ada500c72

git at public.curoverse.com git at public.curoverse.com
Wed May 6 17:12:47 EDT 2015


Summary of changes:
 services/keepstore/handlers.go       | 13 +++++++---
 services/keepstore/keepstore_test.go |  8 +++---
 services/keepstore/volume.go         |  3 ++-
 services/keepstore/volume_test.go    | 18 ++++++++------
 services/keepstore/volume_unix.go    | 48 ++++++++++++++++--------------------
 5 files changed, 49 insertions(+), 41 deletions(-)

       via  e7a2db3fe3446929a481f20e1358337ada500c72 (commit)
       via  b9024e06795906fa74925065cef81d094af95fc0 (commit)
       via  6bd5a4e01a768df4078ddf77862751ea86435e7b (commit)
      from  88b0204247e448ba5f715cb01adba4597bb6aae7 (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 e7a2db3fe3446929a481f20e1358337ada500c72
Merge: 88b0204 b9024e0
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed May 6 17:13:36 2015 -0400

    Merge branch '5748-keepstore-leak' refs #5748


commit b9024e06795906fa74925065cef81d094af95fc0
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed May 6 17:10:35 2015 -0400

    5748: Clean up comments and variable names.

diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go
index b0cbabf..66e0810 100644
--- a/services/keepstore/volume_test.go
+++ b/services/keepstore/volume_test.go
@@ -109,7 +109,7 @@ func (v *MockVolume) Mtime(loc string) (time.Time, error) {
 }
 
 func (v *MockVolume) IndexTo(prefix string, w io.Writer) error {
-	v.gotCall("Index")
+	v.gotCall("IndexTo")
 	for loc, block := range v.Store {
 		if !IsValidLocator(loc) || !strings.HasPrefix(loc, prefix) {
 			continue
diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index 7df0b11..8d23d11 100644
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -218,7 +218,7 @@ func (v *UnixVolume) Status() *VolumeStatus {
 
 // IndexTo writes (to the given Writer) a list of blocks found on this
 // volume which begin with the specified prefix. If the prefix is an
-// empty string, Index writes a complete list of blocks.
+// empty string, IndexTo writes a complete list of blocks.
 //
 // Each block is given in the format
 //
@@ -238,24 +238,22 @@ func (v *UnixVolume) IndexTo(prefix string, w io.Writer) error {
 					v, path, err)
 				return nil
 			}
-			locator := filepath.Base(path)
-			// Skip directories that do not match prefix.
-			// We know there is nothing interesting inside.
+			basename := filepath.Base(path)
 			if info.IsDir() &&
-				!strings.HasPrefix(locator, prefix) &&
-				!strings.HasPrefix(prefix, locator) {
+				!strings.HasPrefix(basename, prefix) &&
+				!strings.HasPrefix(prefix, basename) {
+				// Skip directories that do not match
+				// prefix. We know there is nothing
+				// interesting inside.
 				return filepath.SkipDir
 			}
-			// Skip any file that is not apparently a locator, e.g. .meta files
-			if info.IsDir() || !IsValidLocator(locator) {
-				return nil
-			}
-			// Print filenames beginning with prefix
-			if !strings.HasPrefix(locator, prefix) {
+			if info.IsDir() ||
+				!IsValidLocator(basename) ||
+				!strings.HasPrefix(basename, prefix) {
 				return nil
 			}
 			_, err = fmt.Fprintf(w, "%s+%d %d\n",
-				locator, info.Size(), info.ModTime().Unix())
+				basename, info.Size(), info.ModTime().Unix())
 			return err
 		})
 }

commit 6bd5a4e01a768df4078ddf77862751ea86435e7b
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed May 6 13:36:16 2015 -0400

    5748: Write index data to http.ResponseWriter, instead of using string
    concatenation to buffer the entire response.

diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index d355e92..6492045 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -215,11 +215,18 @@ func IndexHandler(resp http.ResponseWriter, req *http.Request) {
 
 	prefix := mux.Vars(req)["prefix"]
 
-	var index string
 	for _, vol := range KeepVM.AllReadable() {
-		index = index + vol.Index(prefix)
+		if err := vol.IndexTo(prefix, resp); err != nil {
+			// The only errors returned by IndexTo are
+			// write errors returned by resp.Write(),
+			// which probably means the client has
+			// disconnected and this error will never be
+			// reported to the client -- but it will
+			// appear in our own error log.
+			http.Error(resp, err.Error(), http.StatusInternalServerError)
+			return
+		}
 	}
-	resp.Write([]byte(index))
 }
 
 // StatusHandler
diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 8874674..811cc70 100644
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -394,8 +394,10 @@ func TestIndex(t *testing.T) {
 	vols[0].Put(TEST_HASH+".meta", []byte("metadata"))
 	vols[1].Put(TEST_HASH_2+".meta", []byte("metadata"))
 
-	index := vols[0].Index("") + vols[1].Index("")
-	index_rows := strings.Split(index, "\n")
+	buf := new(bytes.Buffer)
+	vols[0].IndexTo("", buf)
+	vols[1].IndexTo("", buf)
+	index_rows := strings.Split(string(buf.Bytes()), "\n")
 	sort.Strings(index_rows)
 	sorted_index := strings.Join(index_rows, "\n")
 	expected := `^\n` + TEST_HASH + `\+\d+ \d+\n` +
@@ -405,7 +407,7 @@ func TestIndex(t *testing.T) {
 	match, err := regexp.MatchString(expected, sorted_index)
 	if err == nil {
 		if !match {
-			t.Errorf("IndexLocators returned:\n%s", index)
+			t.Errorf("IndexLocators returned:\n%s", string(buf.Bytes()))
 		}
 	} else {
 		t.Errorf("regexp.MatchString: %s", err)
diff --git a/services/keepstore/volume.go b/services/keepstore/volume.go
index f581b28..1b52949 100644
--- a/services/keepstore/volume.go
+++ b/services/keepstore/volume.go
@@ -5,6 +5,7 @@
 package main
 
 import (
+	"io"
 	"sync/atomic"
 	"time"
 )
@@ -14,7 +15,7 @@ type Volume interface {
 	Put(loc string, block []byte) error
 	Touch(loc string) error
 	Mtime(loc string) (time.Time, error)
-	Index(prefix string) string
+	IndexTo(prefix string, writer io.Writer) error
 	Delete(loc string) error
 	Status() *VolumeStatus
 	String() string
diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go
index 379c890..b0cbabf 100644
--- a/services/keepstore/volume_test.go
+++ b/services/keepstore/volume_test.go
@@ -3,6 +3,7 @@ package main
 import (
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"strings"
 	"sync"
@@ -107,16 +108,19 @@ func (v *MockVolume) Mtime(loc string) (time.Time, error) {
 	return mtime, err
 }
 
-func (v *MockVolume) Index(prefix string) string {
+func (v *MockVolume) IndexTo(prefix string, w io.Writer) error {
 	v.gotCall("Index")
-	var result string
 	for loc, block := range v.Store {
-		if IsValidLocator(loc) && strings.HasPrefix(loc, prefix) {
-			result = result + fmt.Sprintf("%s+%d %d\n",
-				loc, len(block), 123456789)
+		if !IsValidLocator(loc) || !strings.HasPrefix(loc, prefix) {
+			continue
+		}
+		_, err := fmt.Fprintf(w, "%s+%d %d\n",
+			loc, len(block), 123456789)
+		if err != nil {
+			return err
 		}
 	}
-	return result
+	return nil
 }
 
 func (v *MockVolume) Delete(loc string) error {
diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index 3b7c993..7df0b11 100644
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -4,6 +4,7 @@ package main
 
 import (
 	"fmt"
+	"io"
 	"io/ioutil"
 	"log"
 	"os"
@@ -215,14 +216,13 @@ func (v *UnixVolume) Status() *VolumeStatus {
 	return &VolumeStatus{v.root, devnum, free, used}
 }
 
-// Index returns a list of blocks found on this volume which begin with
-// the specified prefix. If the prefix is an empty string, Index returns
-// a complete list of blocks.
+// IndexTo writes (to the given Writer) a list of blocks found on this
+// volume which begin with the specified prefix. If the prefix is an
+// empty string, Index writes a complete list of blocks.
 //
-// The return value is a multiline string (separated by
-// newlines). Each line is in the format
+// Each block is given in the format
 //
-//     locator+size modification-time
+//     locator+size modification-time {newline}
 //
 // e.g.:
 //
@@ -230,14 +230,11 @@ func (v *UnixVolume) Status() *VolumeStatus {
 //     e4d41e6fd68460e0e3fc18cc746959d2+67108864 1377796043
 //     e4de7a2810f5554cd39b36d8ddb132ff+67108864 1388701136
 //
-func (v *UnixVolume) Index(prefix string) (output string) {
-	filepath.Walk(v.root,
+func (v *UnixVolume) IndexTo(prefix string, w io.Writer) error {
+	return filepath.Walk(v.root,
 		func(path string, info os.FileInfo, err error) error {
-			// This WalkFunc inspects each path in the volume
-			// and prints an index line for all files that begin
-			// with prefix.
 			if err != nil {
-				log.Printf("IndexHandler: %s: walking to %s: %s",
+				log.Printf("%s: IndexTo Walk error at %s: %s",
 					v, path, err)
 				return nil
 			}
@@ -250,18 +247,17 @@ func (v *UnixVolume) Index(prefix string) (output string) {
 				return filepath.SkipDir
 			}
 			// Skip any file that is not apparently a locator, e.g. .meta files
-			if !IsValidLocator(locator) {
+			if info.IsDir() || !IsValidLocator(locator) {
 				return nil
 			}
 			// Print filenames beginning with prefix
-			if !info.IsDir() && strings.HasPrefix(locator, prefix) {
-				output = output + fmt.Sprintf(
-					"%s+%d %d\n", locator, info.Size(), info.ModTime().Unix())
+			if !strings.HasPrefix(locator, prefix) {
+				return nil
 			}
-			return nil
+			_, err = fmt.Fprintf(w, "%s+%d %d\n",
+				locator, info.Size(), info.ModTime().Unix())
+			return err
 		})
-
-	return
 }
 
 func (v *UnixVolume) Delete(loc string) error {

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list