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 d355e925d97a2c2c430c1d304e49286d2f19b592..6492045c68b1f0cbd9de2e00aaa0859ce6ec8b9a 100644 (file)
--- 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 88746745928053aad79e7f481b0c24fc7d741b8a..811cc70d3f75460642a0af60e8138301764b21ad 100644 (file)
--- 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 f581b28fbf897f31badee2f2c4e4100392c5457e..1b5294952820b5cb2f0be2d53c37eb84f2cefed5 100644 (file)
--- 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 379c8903a56a02b2499c2b08cd6c2b800d9a4860..b0cbabf800de5ebd040f805949c9d755997c7161 100644 (file)
--- 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 3b7c9930f780cf0b735016bbaac0b7c243a6d780..7df0b11245d643e4d62e2eaa90239df88769201f 100644 (file)
--- 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 {