[ARVADOS] created: 1.1.0-161-g127e916

Git user git at public.curoverse.com
Mon Nov 20 12:12:58 EST 2017


        at  127e916894bcd16f6978aa6488c81e79a9ca2812 (commit)


commit 127e916894bcd16f6978aa6488c81e79a9ca2812
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Nov 20 12:11:18 2017 -0500

    12483: More loading speed.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 45c0731..4c97af5 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -126,24 +126,25 @@ type fileSystem struct {
 }
 
 func (fs *fileSystem) OpenFile(name string, flag int, perm os.FileMode) (File, error) {
-	return fs.dirnode.OpenFile(path.Clean(name), flag, perm)
+	return fs.dirnode.OpenFile(name, flag, perm)
 }
 
 func (fs *fileSystem) Open(name string) (http.File, error) {
-	return fs.dirnode.OpenFile(path.Clean(name), os.O_RDONLY, 0)
+	return fs.dirnode.OpenFile(name, os.O_RDONLY, 0)
 }
 
 func (fs *fileSystem) Create(name string) (File, error) {
-	return fs.dirnode.OpenFile(path.Clean(name), os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0)
+	return fs.dirnode.OpenFile(name, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0)
 }
 
-func (fs *fileSystem) Stat(name string) (os.FileInfo, error) {
-	f, err := fs.OpenFile(name, os.O_RDONLY, 0)
-	if err != nil {
-		return nil, err
+func (fs *fileSystem) Stat(name string) (fi os.FileInfo, err error) {
+	node := fs.dirnode.lookupPath(name)
+	if node == nil {
+		err = os.ErrNotExist
+	} else {
+		fi = node.Stat()
 	}
-	defer f.Close()
-	return f.Stat()
+	return
 }
 
 type inode interface {
@@ -240,9 +241,8 @@ func (fn *filenode) seek(startPtr filenodePtr) (ptr filenodePtr) {
 	return
 }
 
+// caller must have lock
 func (fn *filenode) appendExtent(e extent) {
-	fn.Lock()
-	defer fn.Unlock()
 	fn.extents = append(fn.extents, e)
 	fn.fileinfo.size += int64(e.Len())
 }
@@ -799,12 +799,13 @@ func (dn *dirnode) loadManifest(txt string) error {
 	if streams[len(streams)-1] != "" {
 		return fmt.Errorf("line %d: no trailing newline", len(streams))
 	}
+	var extents []storedExtent
 	for i, stream := range streams[:len(streams)-1] {
 		lineno := i + 1
-		var extents []storedExtent
 		var anyFileTokens bool
 		var pos int64
 		var extIdx int
+		extents = extents[:0]
 		for i, token := range strings.Split(stream, " ") {
 			if i == 0 {
 				dirname = manifestUnescape(token)
@@ -847,7 +848,7 @@ func (dn *dirnode) loadManifest(txt string) error {
 			if err != nil || length < 0 {
 				return fmt.Errorf("line %d: bad file segment %q", lineno, token)
 			}
-			name := path.Clean(dirname + "/" + manifestUnescape(toks[2]))
+			name := dirname + "/" + manifestUnescape(toks[2])
 			fnode, err := dn.createFileAndParents(name)
 			if err != nil {
 				return fmt.Errorf("line %d: cannot use path %q: %s", lineno, name, err)

commit 415e641036ebcfb944a37ffa14a101f2545104c3
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Nov 20 10:30:05 2017 -0500

    12483: Speed up manifest loading.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 459b476..45c0731 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -803,6 +803,8 @@ func (dn *dirnode) loadManifest(txt string) error {
 		lineno := i + 1
 		var extents []storedExtent
 		var anyFileTokens bool
+		var pos int64
+		var extIdx int
 		for i, token := range strings.Split(stream, " ") {
 			if i == 0 {
 				dirname = manifestUnescape(token)
@@ -846,29 +848,28 @@ func (dn *dirnode) loadManifest(txt string) error {
 				return fmt.Errorf("line %d: bad file segment %q", lineno, token)
 			}
 			name := path.Clean(dirname + "/" + manifestUnescape(toks[2]))
-			err = dn.makeParentDirs(name)
+			fnode, err := dn.createFileAndParents(name)
 			if err != nil {
 				return fmt.Errorf("line %d: cannot use path %q: %s", lineno, name, err)
 			}
-			f, err := dn.OpenFile(name, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0700)
-			if err != nil {
-				return fmt.Errorf("line %d: cannot append to %q: %s", lineno, name, err)
-			}
-			if f.inode.Stat().IsDir() {
-				f.Close()
-				return fmt.Errorf("line %d: cannot append to %q: is a directory", lineno, name)
-			}
 			// Map the stream offset/range coordinates to
 			// block/offset/range coordinates and add
 			// corresponding storedExtents to the filenode
-			var pos int64
-			for _, e := range extents {
-				next := pos + int64(e.Len())
+			if pos > offset {
+				// Can't continue where we left off.
+				// TODO: binary search instead of
+				// rewinding all the way (but this
+				// situation might be rare anyway)
+				extIdx, pos = 0, 0
+			}
+			for next := int64(0); extIdx < len(extents); extIdx, pos = extIdx+1, next {
+				e := extents[extIdx]
+				next = pos + int64(e.Len())
 				if next <= offset || e.Len() == 0 {
 					pos = next
 					continue
 				}
-				if pos > offset+length {
+				if pos >= offset+length {
 					break
 				}
 				var blkOff int
@@ -879,17 +880,18 @@ func (dn *dirnode) loadManifest(txt string) error {
 				if pos+int64(blkOff+blkLen) > offset+length {
 					blkLen = int(offset + length - pos - int64(blkOff))
 				}
-				f.inode.(*filenode).appendExtent(storedExtent{
+				fnode.appendExtent(storedExtent{
 					kc:      dn.kc,
 					locator: e.locator,
 					size:    e.size,
 					offset:  blkOff,
 					length:  blkLen,
 				})
-				pos = next
+				if next > offset+length {
+					break
+				}
 			}
-			f.Close()
-			if pos < offset+length {
+			if extIdx == len(extents) && pos < offset+length {
 				return fmt.Errorf("line %d: invalid segment in %d-byte stream: %q", lineno, pos, token)
 			}
 		}
@@ -904,21 +906,41 @@ func (dn *dirnode) loadManifest(txt string) error {
 	return nil
 }
 
-func (dn *dirnode) makeParentDirs(name string) (err error) {
-	names := strings.Split(name, "/")
-	for _, name := range names[:len(names)-1] {
-		f, err := dn.OpenFile(name, os.O_CREATE, os.ModeDir|0755)
-		if err != nil {
-			return err
+// only safe to call from loadManifest -- no locking
+func (dn *dirnode) createFileAndParents(path string) (fn *filenode, err error) {
+	names := strings.Split(path, "/")
+	if basename := names[len(names)-1]; basename == "" || basename == "." || basename == ".." {
+		err = fmt.Errorf("invalid filename")
+		return
+	}
+	var node inode = dn
+	for i, name := range names {
+		dn, ok := node.(*dirnode)
+		if !ok {
+			err = ErrFileExists
+			return
 		}
-		defer f.Close()
-		var ok bool
-		dn, ok = f.inode.(*dirnode)
+		if name == "" || name == "." {
+			continue
+		}
+		if name == ".." {
+			node = dn.parent
+			continue
+		}
+		node, ok = dn.inodes[name]
 		if !ok {
-			return ErrFileExists
+			if i == len(names)-1 {
+				fn = dn.newFilenode(name, 0755)
+				return
+			}
+			node = dn.newDirnode(name, 0755)
 		}
 	}
-	return nil
+	var ok bool
+	if fn, ok = node.(*filenode); !ok {
+		err = ErrInvalidArgument
+	}
+	return
 }
 
 func (dn *dirnode) mkdir(name string) (*file, error) {
@@ -1103,6 +1125,40 @@ func (dn *dirnode) lookupPath(path string) (node inode) {
 	return
 }
 
+func (dn *dirnode) newDirnode(name string, perm os.FileMode) *dirnode {
+	child := &dirnode{
+		parent: dn,
+		client: dn.client,
+		kc:     dn.kc,
+		fileinfo: fileinfo{
+			name: name,
+			mode: os.ModeDir | perm,
+		},
+	}
+	if dn.inodes == nil {
+		dn.inodes = make(map[string]inode)
+	}
+	dn.inodes[name] = child
+	dn.fileinfo.size++
+	return child
+}
+
+func (dn *dirnode) newFilenode(name string, perm os.FileMode) *filenode {
+	child := &filenode{
+		parent: dn,
+		fileinfo: fileinfo{
+			name: name,
+			mode: perm,
+		},
+	}
+	if dn.inodes == nil {
+		dn.inodes = make(map[string]inode)
+	}
+	dn.inodes[name] = child
+	dn.fileinfo.size++
+	return child
+}
+
 // OpenFile is analogous to os.OpenFile().
 func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, error) {
 	if flag&os.O_SYNC != 0 {
@@ -1149,29 +1205,10 @@ func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, err
 			return nil, os.ErrNotExist
 		}
 		if perm.IsDir() {
-			n = &dirnode{
-				parent: dn,
-				client: dn.client,
-				kc:     dn.kc,
-				fileinfo: fileinfo{
-					name: name,
-					mode: os.ModeDir | 0755,
-				},
-			}
+			n = dn.newDirnode(name, 0755)
 		} else {
-			n = &filenode{
-				parent: dn,
-				fileinfo: fileinfo{
-					name: name,
-					mode: 0755,
-				},
-			}
-		}
-		if dn.inodes == nil {
-			dn.inodes = make(map[string]inode)
+			n = dn.newFilenode(name, 0755)
 		}
-		dn.inodes[name] = n
-		dn.fileinfo.size++
 	} else if flag&os.O_EXCL != 0 {
 		return nil, ErrFileExists
 	} else if flag&os.O_TRUNC != 0 {
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 07714eb..07d00be 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -5,6 +5,7 @@
 package arvados
 
 import (
+	"bytes"
 	"crypto/md5"
 	"errors"
 	"fmt"
@@ -14,8 +15,10 @@ import (
 	"net/http"
 	"os"
 	"regexp"
+	"runtime"
 	"sync"
 	"testing"
+	"time"
 
 	"git.curoverse.com/arvados.git/sdk/go/arvadostest"
 	check "gopkg.in/check.v1"
@@ -964,6 +967,50 @@ func (s *CollectionFSSuite) checkMemSize(c *check.C, f File) {
 	c.Check(fn.memsize, check.Equals, memsize)
 }
 
+type CollectionFSUnitSuite struct{}
+
+var _ = check.Suite(&CollectionFSUnitSuite{})
+
+// expect ~2 seconds to load a manifest with 256K files
+func (s *CollectionFSUnitSuite) TestLargeManifest(c *check.C) {
+	const (
+		dirCount  = 512
+		fileCount = 512
+	)
+
+	mb := bytes.NewBuffer(make([]byte, 0, 40000000))
+	for i := 0; i < dirCount; i++ {
+		fmt.Fprintf(mb, "./dir%d", i)
+		for j := 0; j <= fileCount; j++ {
+			fmt.Fprintf(mb, " %032x+42+A%040x@%08x", j, j, j)
+		}
+		for j := 0; j < fileCount; j++ {
+			fmt.Fprintf(mb, " %d:%d:dir%d/file%d", j*42+21, 42, j, j)
+		}
+		mb.Write([]byte{'\n'})
+	}
+	coll := Collection{ManifestText: mb.String()}
+	c.Logf("%s built", time.Now())
+
+	var memstats runtime.MemStats
+	runtime.ReadMemStats(&memstats)
+	c.Logf("%s Alloc=%d Sys=%d", time.Now(), memstats.Alloc, memstats.Sys)
+
+	f, err := coll.FileSystem(nil, nil)
+	c.Check(err, check.IsNil)
+	c.Logf("%s loaded", time.Now())
+
+	for i := 0; i < dirCount; i++ {
+		for j := 0; j < fileCount; j++ {
+			f.Stat(fmt.Sprintf("./dir%d/dir%d/file%d", i, j, j))
+		}
+	}
+	c.Logf("%s Stat() x %d", time.Now(), dirCount*fileCount)
+
+	runtime.ReadMemStats(&memstats)
+	c.Logf("%s Alloc=%d Sys=%d", time.Now(), memstats.Alloc, memstats.Sys)
+}
+
 // Gocheck boilerplate
 func Test(t *testing.T) {
 	check.TestingT(t)

commit 33d4ec966d17ffc98034504c308969dbe9177a85
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sat Nov 18 01:44:55 2017 -0500

    12483: Allow seek and write beyond EOF.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index fd524c9..459b476 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -200,7 +200,6 @@ func (fn *filenode) seek(startPtr filenodePtr) (ptr filenodePtr) {
 		// meaningless anyway
 		return
 	} else if ptr.off >= fn.fileinfo.size {
-		ptr.off = fn.fileinfo.size
 		ptr.extentIdx = len(fn.extents)
 		ptr.extentOff = 0
 		ptr.repacked = fn.repacked
@@ -296,8 +295,16 @@ func (fn *filenode) Stat() os.FileInfo {
 func (fn *filenode) Truncate(size int64) error {
 	fn.Lock()
 	defer fn.Unlock()
+	return fn.truncate(size)
+}
+
+func (fn *filenode) truncate(size int64) error {
+	if size == fn.fileinfo.size {
+		return nil
+	}
+	fn.repacked++
 	if size < fn.fileinfo.size {
-		ptr := fn.seek(filenodePtr{off: size, repacked: fn.repacked - 1})
+		ptr := fn.seek(filenodePtr{off: size})
 		for i := ptr.extentIdx; i < len(fn.extents); i++ {
 			if ext, ok := fn.extents[i].(*memExtent); ok {
 				fn.memsize -= int64(ext.Len())
@@ -316,7 +323,6 @@ func (fn *filenode) Truncate(size int64) error {
 			}
 		}
 		fn.fileinfo.size = size
-		fn.repacked++
 		return nil
 	}
 	for size > fn.fileinfo.size {
@@ -329,8 +335,6 @@ func (fn *filenode) Truncate(size int64) error {
 		} else if e, ok = fn.extents[len(fn.extents)-1].(writableExtent); !ok || e.Len() >= maxBlockSize {
 			e = &memExtent{}
 			fn.extents = append(fn.extents, e)
-		} else {
-			fn.repacked++
 		}
 		if maxgrow := int64(maxBlockSize - e.Len()); maxgrow < grow {
 			grow = maxgrow
@@ -342,7 +346,13 @@ func (fn *filenode) Truncate(size int64) error {
 	return nil
 }
 
+// Caller must hold lock.
 func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr, err error) {
+	if startPtr.off > fn.fileinfo.size {
+		if err = fn.truncate(startPtr.off); err != nil {
+			return 0, startPtr, err
+		}
+	}
 	ptr = fn.seek(startPtr)
 	if ptr.off < 0 {
 		err = ErrNegativeOffset
@@ -548,9 +558,6 @@ func (f *file) Seek(off int64, whence int) (pos int64, err error) {
 	if ptr.off < 0 {
 		return f.ptr.off, ErrNegativeOffset
 	}
-	if ptr.off > size {
-		ptr.off = size
-	}
 	if ptr.off != f.ptr.off {
 		f.ptr = ptr
 		// force filenode to recompute f.ptr fields on next
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 5ef357f..07714eb 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -281,6 +281,7 @@ func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
 	c.Check(string(buf2), check.Equals, "f0123456789abcd\x00\x00\x00\x00\x00")
 
 	f.Truncate(0)
+	f2.Seek(0, os.SEEK_SET)
 	f2.Write([]byte("12345678abcdefghijkl"))
 
 	// grow to block/extent boundary
@@ -330,6 +331,46 @@ func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
 	c.Check(m, check.Equals, "./dir1 3858f62230ac3c915f300c664312c63f+6 25d55ad283aa400af464c76d713c07ad+8 3:3:bar 6:3:foo\n")
 }
 
+func (s *CollectionFSSuite) TestSeekSparse(c *check.C) {
+	fs, err := (&Collection{}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
+	f, err := fs.OpenFile("test", os.O_CREATE|os.O_RDWR, 0755)
+	c.Assert(err, check.IsNil)
+	defer f.Close()
+
+	checkSize := func(size int64) {
+		fi, err := f.Stat()
+		c.Check(fi.Size(), check.Equals, size)
+
+		f, err := fs.OpenFile("test", os.O_CREATE|os.O_RDWR, 0755)
+		c.Assert(err, check.IsNil)
+		defer f.Close()
+		fi, err = f.Stat()
+		c.Check(fi.Size(), check.Equals, size)
+		pos, err := f.Seek(0, os.SEEK_END)
+		c.Check(pos, check.Equals, size)
+	}
+
+	f.Seek(2, os.SEEK_END)
+	checkSize(0)
+	f.Write([]byte{1})
+	checkSize(3)
+
+	f.Seek(2, os.SEEK_CUR)
+	checkSize(3)
+	f.Write([]byte{})
+	checkSize(5)
+
+	f.Seek(8, os.SEEK_SET)
+	checkSize(5)
+	n, err := f.Read(make([]byte, 1))
+	c.Check(n, check.Equals, 0)
+	c.Check(err, check.Equals, io.EOF)
+	checkSize(5)
+	f.Write([]byte{1, 2, 3})
+	checkSize(11)
+}
+
 func (s *CollectionFSSuite) TestMarshalSmallBlocks(c *check.C) {
 	maxBlockSize = 8
 	defer func() { maxBlockSize = 2 << 26 }()

commit 228699abb423444293dc74e925b99211a3d541a3
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sat Nov 18 00:19:56 2017 -0500

    12483: Avoid empty segments.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 367f6d0..fd524c9 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -857,7 +857,7 @@ func (dn *dirnode) loadManifest(txt string) error {
 			var pos int64
 			for _, e := range extents {
 				next := pos + int64(e.Len())
-				if next < offset {
+				if next <= offset || e.Len() == 0 {
 					pos = next
 					continue
 				}

commit d09c6f1c05bc2a6c4b40c19b8317a03fdb0c232c
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Nov 17 20:53:38 2017 -0500

    12483: Add RemoveAll() to CollectionFileSystem.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index aa549e1..367f6d0 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -116,6 +116,7 @@ type CollectionFileSystem interface {
 
 	Mkdir(name string, perm os.FileMode) error
 	Remove(name string) error
+	RemoveAll(name string) error
 	Rename(oldname, newname string) error
 	MarshalManifest(prefix string) (string, error)
 }
@@ -926,9 +927,17 @@ func (dn *dirnode) Mkdir(name string, perm os.FileMode) error {
 }
 
 func (dn *dirnode) Remove(name string) error {
+	return dn.remove(name, false)
+}
+
+func (dn *dirnode) RemoveAll(name string) error {
+	return dn.remove(name, true)
+}
+
+func (dn *dirnode) remove(name string, recursive bool) error {
 	dirname, name := path.Split(name)
 	if name == "" || name == "." || name == ".." {
-		return ErrInvalidOperation
+		return ErrInvalidArgument
 	}
 	dn, ok := dn.lookupPath(dirname).(*dirnode)
 	if !ok {
@@ -942,7 +951,7 @@ func (dn *dirnode) Remove(name string) error {
 	case *dirnode:
 		node.RLock()
 		defer node.RUnlock()
-		if len(node.inodes) > 0 {
+		if !recursive && len(node.inodes) > 0 {
 			return ErrDirectoryNotEmpty
 		}
 	}
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 28f5a34..5ef357f 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -507,6 +507,37 @@ func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
 	// TODO: check manifest content
 }
 
+func (s *CollectionFSSuite) TestRemove(c *check.C) {
+	fs, err := (&Collection{}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
+	err = fs.Mkdir("dir0", 0755)
+	c.Assert(err, check.IsNil)
+	err = fs.Mkdir("dir1", 0755)
+	c.Assert(err, check.IsNil)
+	err = fs.Mkdir("dir1/dir2", 0755)
+	c.Assert(err, check.IsNil)
+
+	err = fs.Remove("dir0")
+	c.Check(err, check.IsNil)
+	err = fs.Remove("dir0")
+	c.Check(err, check.Equals, os.ErrNotExist)
+
+	err = fs.Remove("dir1/dir2/")
+	c.Check(err, check.Equals, ErrInvalidArgument)
+	err = fs.Remove("dir1/dir2/.")
+	c.Check(err, check.Equals, ErrInvalidArgument)
+	err = fs.Remove("dir1/dir2/..")
+	c.Check(err, check.Equals, ErrInvalidArgument)
+	err = fs.Remove("dir1")
+	c.Check(err, check.Equals, ErrDirectoryNotEmpty)
+	err = fs.Remove("dir1/dir2/../../../dir1")
+	c.Check(err, check.Equals, ErrDirectoryNotEmpty)
+	err = fs.RemoveAll("dir1")
+	c.Check(err, check.IsNil)
+	err = fs.RemoveAll("dir1")
+	c.Check(err, check.Equals, os.ErrNotExist)
+}
+
 func (s *CollectionFSSuite) TestRename(c *check.C) {
 	fs, err := (&Collection{}).FileSystem(s.client, s.kc)
 	c.Assert(err, check.IsNil)
diff --git a/services/keep-web/webdav.go b/services/keep-web/webdav.go
index 4a9476d..7798a69 100644
--- a/services/keep-web/webdav.go
+++ b/services/keep-web/webdav.go
@@ -49,14 +49,14 @@ func (fs *webdavFS) OpenFile(ctx context.Context, name string, flag int, perm os
 }
 
 func (fs *webdavFS) RemoveAll(ctx context.Context, name string) error {
-	return errReadOnly
+	return fs.collfs.RemoveAll(name)
 }
 
 func (fs *webdavFS) Rename(ctx context.Context, oldName, newName string) error {
 	if fs.update == nil {
 		return errReadOnly
 	}
-	return fs.Rename(oldName, newName)
+	return fs.collfs.Rename(oldName, newName)
 }
 
 func (fs *webdavFS) Stat(ctx context.Context, name string) (os.FileInfo, error) {

commit e789dc0f82a18f7e5830bc1a3647b353035814db
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Nov 17 11:38:13 2017 -0500

    12483: Enable rename via webdav.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 42f6c51..f6c12c3 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -98,6 +98,7 @@ func (h *handler) serveStatus(w http.ResponseWriter, r *http.Request) {
 
 var (
 	webdavMethod = map[string]bool{
+		"MOVE":     true,
 		"OPTIONS":  true,
 		"PROPFIND": true,
 		"PUT":      true,
@@ -148,7 +149,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 			return
 		}
 		w.Header().Set("Access-Control-Allow-Headers", "Authorization, Content-Type, Range")
-		w.Header().Set("Access-Control-Allow-Methods", "GET, POST, OPTIONS, PROPFIND, PUT")
+		w.Header().Set("Access-Control-Allow-Methods", "GET, MOVE, OPTIONS, POST, PROPFIND, PUT")
 		w.Header().Set("Access-Control-Allow-Origin", "*")
 		w.Header().Set("Access-Control-Max-Age", "86400")
 		statusCode = http.StatusOK
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 32174cb..6add804 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -45,7 +45,7 @@ func (s *UnitSuite) TestCORSPreflight(c *check.C) {
 	c.Check(resp.Code, check.Equals, http.StatusOK)
 	c.Check(resp.Body.String(), check.Equals, "")
 	c.Check(resp.Header().Get("Access-Control-Allow-Origin"), check.Equals, "*")
-	c.Check(resp.Header().Get("Access-Control-Allow-Methods"), check.Equals, "GET, POST, OPTIONS, PROPFIND, PUT")
+	c.Check(resp.Header().Get("Access-Control-Allow-Methods"), check.Equals, "GET, MOVE, OPTIONS, POST, PROPFIND, PUT")
 	c.Check(resp.Header().Get("Access-Control-Allow-Headers"), check.Equals, "Authorization, Content-Type, Range")
 
 	// Check preflight for a disallowed request
diff --git a/services/keep-web/webdav.go b/services/keep-web/webdav.go
index 87d9c47..4a9476d 100644
--- a/services/keep-web/webdav.go
+++ b/services/keep-web/webdav.go
@@ -53,7 +53,10 @@ func (fs *webdavFS) RemoveAll(ctx context.Context, name string) error {
 }
 
 func (fs *webdavFS) Rename(ctx context.Context, oldName, newName string) error {
-	return errReadOnly
+	if fs.update == nil {
+		return errReadOnly
+	}
+	return fs.Rename(oldName, newName)
 }
 
 func (fs *webdavFS) Stat(ctx context.Context, name string) (os.FileInfo, error) {

commit 00a4d2f890cd363725a4d6697bbf92498c866420
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Nov 17 11:06:24 2017 -0500

    12483: Add Rename(old,new) to CollectionFileSystem.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 6628a75..aa549e1 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -24,9 +24,11 @@ var (
 	ErrNegativeOffset    = errors.New("cannot seek to negative offset")
 	ErrFileExists        = errors.New("file exists")
 	ErrInvalidOperation  = errors.New("invalid operation")
+	ErrInvalidArgument   = errors.New("invalid argument")
 	ErrDirectoryNotEmpty = errors.New("directory not empty")
 	ErrWriteOnlyMode     = errors.New("file is O_WRONLY")
 	ErrSyncNotSupported  = errors.New("O_SYNC flag is not supported")
+	ErrIsDirectory       = errors.New("cannot rename file to overwrite existing directory")
 	ErrPermission        = os.ErrPermission
 
 	maxBlockSize = 1 << 26
@@ -114,6 +116,7 @@ type CollectionFileSystem interface {
 
 	Mkdir(name string, perm os.FileMode) error
 	Remove(name string) error
+	Rename(oldname, newname string) error
 	MarshalManifest(prefix string) (string, error)
 }
 
@@ -947,6 +950,79 @@ func (dn *dirnode) Remove(name string) error {
 	return nil
 }
 
+func (dn *dirnode) Rename(oldname, newname string) error {
+	olddir, oldname := path.Split(oldname)
+	if oldname == "" || oldname == "." || oldname == ".." {
+		return ErrInvalidArgument
+	}
+	olddirf, err := dn.OpenFile(olddir+".", os.O_RDONLY, 0)
+	if err != nil {
+		return fmt.Errorf("%q: %s", olddir, err)
+	}
+	defer olddirf.Close()
+	newdir, newname := path.Split(newname)
+	if newname == "." || newname == ".." {
+		return ErrInvalidArgument
+	} else if newname == "" {
+		// Rename("a/b", "c/") means Rename("a/b", "c/b")
+		newname = oldname
+	}
+	newdirf, err := dn.OpenFile(newdir+".", os.O_RDONLY, 0)
+	if err != nil {
+		return fmt.Errorf("%q: %s", newdir, err)
+	}
+	defer newdirf.Close()
+
+	// When acquiring locks on multiple nodes, all common
+	// ancestors must be locked first in order to avoid
+	// deadlock. This is assured by locking the path from root to
+	// newdir, then locking the path from root to olddir, skipping
+	// any already-locked nodes.
+	needLock := []sync.Locker{}
+	for _, f := range []*file{olddirf, newdirf} {
+		node := f.inode
+		needLock = append(needLock, node)
+		for node.Parent() != node {
+			node = node.Parent()
+			needLock = append(needLock, node)
+		}
+	}
+	locked := map[sync.Locker]bool{}
+	for i := len(needLock) - 1; i >= 0; i-- {
+		if n := needLock[i]; !locked[n] {
+			n.Lock()
+			defer n.Unlock()
+			locked[n] = true
+		}
+	}
+
+	olddn := olddirf.inode.(*dirnode)
+	newdn := newdirf.inode.(*dirnode)
+	oldinode, ok := olddn.inodes[oldname]
+	if !ok {
+		return os.ErrNotExist
+	}
+	if existing, ok := newdn.inodes[newname]; ok {
+		// overwriting an existing file or dir
+		if dn, ok := existing.(*dirnode); ok {
+			if !oldinode.Stat().IsDir() {
+				return ErrIsDirectory
+			}
+			dn.RLock()
+			defer dn.RUnlock()
+			if len(dn.inodes) > 0 {
+				return ErrDirectoryNotEmpty
+			}
+		}
+	} else {
+		newdn.fileinfo.size++
+	}
+	newdn.inodes[newname] = oldinode
+	delete(olddn.inodes, oldname)
+	olddn.fileinfo.size--
+	return nil
+}
+
 func (dn *dirnode) Parent() inode {
 	dn.RLock()
 	defer dn.RUnlock()
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index fc3af34..28f5a34 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -507,6 +507,89 @@ func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
 	// TODO: check manifest content
 }
 
+func (s *CollectionFSSuite) TestRename(c *check.C) {
+	fs, err := (&Collection{}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
+	const (
+		outer = 16
+		inner = 16
+	)
+	for i := 0; i < outer; i++ {
+		err = fs.Mkdir(fmt.Sprintf("dir%d", i), 0755)
+		c.Assert(err, check.IsNil)
+		for j := 0; j < inner; j++ {
+			err = fs.Mkdir(fmt.Sprintf("dir%d/dir%d", i, j), 0755)
+			c.Assert(err, check.IsNil)
+			for _, fnm := range []string{
+				fmt.Sprintf("dir%d/file%d", i, j),
+				fmt.Sprintf("dir%d/dir%d/file%d", i, j, j),
+			} {
+				f, err := fs.OpenFile(fnm, os.O_CREATE|os.O_WRONLY, 0755)
+				c.Assert(err, check.IsNil)
+				_, err = f.Write([]byte("beep"))
+				c.Assert(err, check.IsNil)
+				f.Close()
+			}
+		}
+	}
+	var wg sync.WaitGroup
+	for i := 0; i < outer; i++ {
+		for j := 0; j < inner; j++ {
+			wg.Add(1)
+			go func(i, j int) {
+				defer wg.Done()
+				oldname := fmt.Sprintf("dir%d/dir%d/file%d", i, j, j)
+				newname := fmt.Sprintf("dir%d/newfile%d", i, inner-j-1)
+				_, err := fs.Open(newname)
+				c.Check(err, check.Equals, os.ErrNotExist)
+				err = fs.Rename(oldname, newname)
+				c.Check(err, check.IsNil)
+				f, err := fs.Open(newname)
+				c.Check(err, check.IsNil)
+				f.Close()
+			}(i, j)
+
+			wg.Add(1)
+			go func(i, j int) {
+				defer wg.Done()
+				// oldname does not exist
+				err := fs.Rename(
+					fmt.Sprintf("dir%d/dir%d/missing", i, j),
+					fmt.Sprintf("dir%d/irelevant", outer-i-1))
+				c.Check(err, check.ErrorMatches, `.*does not exist`)
+
+				// newname parent dir does not exist
+				err = fs.Rename(
+					fmt.Sprintf("dir%d/dir%d", i, j),
+					fmt.Sprintf("dir%d/missing/irrelevant", outer-i-1))
+				c.Check(err, check.ErrorMatches, `.*does not exist`)
+
+				// oldname parent dir is a file
+				err = fs.Rename(
+					fmt.Sprintf("dir%d/file%d/patherror", i, j),
+					fmt.Sprintf("dir%d/irrelevant", i))
+				c.Check(err, check.ErrorMatches, `.*does not exist`)
+
+				// newname parent dir is a file
+				err = fs.Rename(
+					fmt.Sprintf("dir%d/dir%d/file%d", i, j, j),
+					fmt.Sprintf("dir%d/file%d/patherror", i, inner-j-1))
+				c.Check(err, check.ErrorMatches, `.*does not exist`)
+			}(i, j)
+		}
+	}
+	wg.Wait()
+
+	f, err := fs.OpenFile("dir1/newfile3", 0, 0)
+	c.Assert(err, check.IsNil)
+	c.Check(f.Size(), check.Equals, int64(4))
+	buf, err := ioutil.ReadAll(f)
+	c.Check(buf, check.DeepEquals, []byte("beep"))
+	c.Check(err, check.IsNil)
+	_, err = fs.Open("dir1/dir1/file1")
+	c.Check(err, check.Equals, os.ErrNotExist)
+}
+
 func (s *CollectionFSSuite) TestPersist(c *check.C) {
 	maxBlockSize = 1024
 	defer func() { maxBlockSize = 2 << 26 }()
@@ -626,7 +709,7 @@ func (s *CollectionFSSuite) TestPersistEmptyFiles(c *check.C) {
 	}
 }
 
-func (s *CollectionFSSuite) TestFileModes(c *check.C) {
+func (s *CollectionFSSuite) TestOpenFileFlags(c *check.C) {
 	fs, err := (&Collection{}).FileSystem(s.client, s.kc)
 	c.Assert(err, check.IsNil)
 

commit 14f466699a77923bb312695b17a7c8e7c0d17ed9
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Nov 17 03:04:00 2017 -0500

    12483: Add comments.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index e720b93..6628a75 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -86,15 +86,35 @@ func (fi fileinfo) Sys() interface{} {
 }
 
 // A CollectionFileSystem is an http.Filesystem plus Stat() and
-// support for opening writable files.
+// support for opening writable files. All methods are safe to call
+// from multiple goroutines.
 type CollectionFileSystem interface {
 	http.FileSystem
+
+	// analogous to os.Stat()
 	Stat(name string) (os.FileInfo, error)
+
+	// analogous to os.Create(): create/truncate a file and open it O_RDWR.
 	Create(name string) (File, error)
+
+	// Like os.OpenFile(): create or open a file or directory.
+	//
+	// If flag&os.O_EXCL==0, it opens an existing file or
+	// directory if one exists. If flag&os.O_CREATE!=0, it creates
+	// a new empty file or directory if one does not already
+	// exist.
+	//
+	// When creating a new item, perm&os.ModeDir determines
+	// whether it is a file or a directory.
+	//
+	// A file can be opened multiple times and used concurrently
+	// from multiple goroutines. However, each File object should
+	// be used by only one goroutine at a time.
 	OpenFile(name string, flag int, perm os.FileMode) (File, error)
+
 	Mkdir(name string, perm os.FileMode) error
 	Remove(name string) error
-	MarshalManifest(string) (string, error)
+	MarshalManifest(prefix string) (string, error)
 }
 
 type fileSystem struct {
@@ -991,6 +1011,7 @@ func (dn *dirnode) lookupPath(path string) (node inode) {
 	return
 }
 
+// OpenFile is analogous to os.OpenFile().
 func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, error) {
 	if flag&os.O_SYNC != 0 {
 		return nil, ErrSyncNotSupported

commit fdc739318a2f0ec44ad3da69465221b48d30c29c
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Thu Nov 16 16:58:18 2017 -0500

    12483: Support O_TRUNC flag.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index beddb92..e720b93 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -1061,6 +1061,14 @@ func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, err
 		dn.fileinfo.size++
 	} else if flag&os.O_EXCL != 0 {
 		return nil, ErrFileExists
+	} else if flag&os.O_TRUNC != 0 {
+		if !writable {
+			return nil, fmt.Errorf("invalid flag O_TRUNC in read-only mode")
+		} else if fn, ok := n.(*filenode); !ok {
+			return nil, fmt.Errorf("invalid flag O_TRUNC when opening directory")
+		} else {
+			fn.Truncate(0)
+		}
 	}
 	return &file{
 		inode:    n,
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 6e8e615..fc3af34 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -641,12 +641,26 @@ func (s *CollectionFSSuite) TestFileModes(c *check.C) {
 	c.Check(n, check.Equals, 0)
 	c.Check(err, check.ErrorMatches, `read-only file`)
 	n, err = f.Read(make([]byte, 1))
+	c.Check(n, check.Equals, 0)
 	c.Check(err, check.Equals, io.EOF)
 	f, err = fs.OpenFile("new", os.O_RDWR, 0)
 	c.Assert(err, check.IsNil)
 	defer f.Close()
 	_, err = f.Write([]byte{4, 5, 6})
 	c.Check(err, check.IsNil)
+	fi, err := f.Stat()
+	c.Assert(err, check.IsNil)
+	c.Check(fi.Size(), check.Equals, int64(3))
+
+	f, err = fs.OpenFile("new", os.O_TRUNC|os.O_RDWR, 0)
+	c.Assert(err, check.IsNil)
+	defer f.Close()
+	pos, err := f.Seek(0, os.SEEK_END)
+	c.Check(pos, check.Equals, int64(0))
+	c.Check(err, check.IsNil)
+	fi, err = f.Stat()
+	c.Assert(err, check.IsNil)
+	c.Check(fi.Size(), check.Equals, int64(0))
 	fs.Remove("new")
 
 	buf := make([]byte, 64)
@@ -657,7 +671,7 @@ func (s *CollectionFSSuite) TestFileModes(c *check.C) {
 	n, _ = f.Read(buf[:1])
 	c.Check(n, check.Equals, 1)
 	c.Check(buf[:1], check.DeepEquals, []byte{1})
-	pos, err := f.Seek(0, os.SEEK_CUR)
+	pos, err = f.Seek(0, os.SEEK_CUR)
 	c.Check(pos, check.Equals, int64(1))
 	f.Write([]byte{4, 5, 6})
 	pos, err = f.Seek(0, os.SEEK_CUR)

commit 5759c046e8d53818689774abde541449aeb3e0fb
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Thu Nov 16 16:19:58 2017 -0500

    12483: Compress adjacent segments when writing filetokens.
    
    "0:100:foo 100:100:foo 200:100:foo" --> "0:300:foo"
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 3cae9cb..beddb92 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -725,11 +725,18 @@ func (dn *dirnode) marshalManifest(prefix string) (string, error) {
 					} else {
 						blocks = append(blocks, e.locator)
 					}
-					segments = append(segments, m1segment{
+					next := m1segment{
 						name:   name,
 						offset: streamLen + int64(e.offset),
 						length: int64(e.length),
-					})
+					}
+					if prev := len(segments) - 1; prev >= 0 &&
+						segments[prev].name == name &&
+						segments[prev].offset+segments[prev].length == next.offset {
+						segments[prev].length += next.length
+					} else {
+						segments = append(segments, next)
+					}
 					streamLen += int64(e.size)
 				default:
 					// This can't happen: we
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 324ece1..6e8e615 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -738,7 +738,7 @@ func (s *CollectionFSSuite) TestFlushFullBlocks(c *check.C) {
 	c.Check(currentMemExtents(), check.HasLen, 1)
 
 	m, err := fs.MarshalManifest(".")
-	c.Check(m, check.Not(check.Equals), "")
+	c.Check(m, check.Matches, `[^:]* 0:50000:50K\n`)
 	c.Check(err, check.IsNil)
 	c.Check(currentMemExtents(), check.HasLen, 0)
 }

commit 0373c781e9fdc31246b64e122a2cf561287d16d6
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Thu Nov 16 16:06:20 2017 -0500

    12483: Support O_APPEND. Check for invalid/unsupported file modes.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 328d68f..3cae9cb 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -25,6 +25,8 @@ var (
 	ErrFileExists        = errors.New("file exists")
 	ErrInvalidOperation  = errors.New("invalid operation")
 	ErrDirectoryNotEmpty = errors.New("directory not empty")
+	ErrWriteOnlyMode     = errors.New("file is O_WRONLY")
+	ErrSyncNotSupported  = errors.New("O_SYNC flag is not supported")
 	ErrPermission        = os.ErrPermission
 
 	maxBlockSize = 1 << 26
@@ -231,8 +233,6 @@ func (fn *filenode) Readdir() []os.FileInfo {
 }
 
 func (fn *filenode) Read(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr, err error) {
-	fn.RLock()
-	defer fn.RUnlock()
 	ptr = fn.seek(startPtr)
 	if ptr.off < 0 {
 		err = ErrNegativeOffset
@@ -319,8 +319,6 @@ func (fn *filenode) Truncate(size int64) error {
 }
 
 func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr, err error) {
-	fn.Lock()
-	defer fn.Unlock()
 	ptr = fn.seek(startPtr)
 	if ptr.off < 0 {
 		err = ErrNegativeOffset
@@ -497,11 +495,17 @@ type file struct {
 	inode
 	ptr        filenodePtr
 	append     bool
+	readable   bool
 	writable   bool
 	unreaddirs []os.FileInfo
 }
 
 func (f *file) Read(p []byte) (n int, err error) {
+	if !f.readable {
+		return 0, ErrWriteOnlyMode
+	}
+	f.inode.RLock()
+	defer f.inode.RUnlock()
 	n, f.ptr, err = f.inode.Read(p, f.ptr)
 	return
 }
@@ -540,6 +544,16 @@ func (f *file) Write(p []byte) (n int, err error) {
 	if !f.writable {
 		return 0, ErrReadOnlyFile
 	}
+	f.inode.Lock()
+	defer f.inode.Unlock()
+	if fn, ok := f.inode.(*filenode); ok && f.append {
+		f.ptr = filenodePtr{
+			off:       fn.fileinfo.size,
+			extentIdx: len(fn.extents),
+			extentOff: 0,
+			repacked:  fn.repacked,
+		}
+	}
 	n, f.ptr, err = f.inode.Write(p, f.ptr)
 	return
 }
@@ -971,13 +985,27 @@ func (dn *dirnode) lookupPath(path string) (node inode) {
 }
 
 func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, error) {
+	if flag&os.O_SYNC != 0 {
+		return nil, ErrSyncNotSupported
+	}
 	dirname, name := path.Split(name)
 	dn, ok := dn.lookupPath(dirname).(*dirnode)
 	if !ok {
 		return nil, os.ErrNotExist
 	}
-	writeMode := flag&(os.O_RDWR|os.O_WRONLY|os.O_CREATE) != 0
-	if !writeMode {
+	var readable, writable bool
+	switch flag & (os.O_RDWR | os.O_RDONLY | os.O_WRONLY) {
+	case os.O_RDWR:
+		readable = true
+		writable = true
+	case os.O_RDONLY:
+		readable = true
+	case os.O_WRONLY:
+		writable = true
+	default:
+		return nil, fmt.Errorf("invalid flags 0x%x", flag)
+	}
+	if !writable {
 		// A directory can be opened via "foo/", "foo/.", or
 		// "foo/..".
 		switch name {
@@ -1030,7 +1058,8 @@ func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, err
 	return &file{
 		inode:    n,
 		append:   flag&os.O_APPEND != 0,
-		writable: flag&(os.O_WRONLY|os.O_RDWR) != 0,
+		readable: readable,
+		writable: writable,
 	}, nil
 }
 
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index ecb1f89..324ece1 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -626,6 +626,87 @@ func (s *CollectionFSSuite) TestPersistEmptyFiles(c *check.C) {
 	}
 }
 
+func (s *CollectionFSSuite) TestFileModes(c *check.C) {
+	fs, err := (&Collection{}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
+
+	f, err := fs.OpenFile("missing", os.O_WRONLY, 0)
+	c.Check(f, check.IsNil)
+	c.Check(err, check.ErrorMatches, `file does not exist`)
+
+	f, err = fs.OpenFile("new", os.O_CREATE|os.O_RDONLY, 0)
+	c.Assert(err, check.IsNil)
+	defer f.Close()
+	n, err := f.Write([]byte{1, 2, 3})
+	c.Check(n, check.Equals, 0)
+	c.Check(err, check.ErrorMatches, `read-only file`)
+	n, err = f.Read(make([]byte, 1))
+	c.Check(err, check.Equals, io.EOF)
+	f, err = fs.OpenFile("new", os.O_RDWR, 0)
+	c.Assert(err, check.IsNil)
+	defer f.Close()
+	_, err = f.Write([]byte{4, 5, 6})
+	c.Check(err, check.IsNil)
+	fs.Remove("new")
+
+	buf := make([]byte, 64)
+	f, err = fs.OpenFile("append", os.O_EXCL|os.O_CREATE|os.O_RDWR|os.O_APPEND, 0)
+	c.Assert(err, check.IsNil)
+	f.Write([]byte{1, 2, 3})
+	f.Seek(0, os.SEEK_SET)
+	n, _ = f.Read(buf[:1])
+	c.Check(n, check.Equals, 1)
+	c.Check(buf[:1], check.DeepEquals, []byte{1})
+	pos, err := f.Seek(0, os.SEEK_CUR)
+	c.Check(pos, check.Equals, int64(1))
+	f.Write([]byte{4, 5, 6})
+	pos, err = f.Seek(0, os.SEEK_CUR)
+	c.Check(pos, check.Equals, int64(6))
+	f.Seek(0, os.SEEK_SET)
+	n, err = f.Read(buf)
+	c.Check(buf[:n], check.DeepEquals, []byte{1, 2, 3, 4, 5, 6})
+	c.Check(err, check.Equals, io.EOF)
+	f.Close()
+
+	f, err = fs.OpenFile("append", os.O_RDWR|os.O_APPEND, 0)
+	c.Assert(err, check.IsNil)
+	pos, err = f.Seek(0, os.SEEK_CUR)
+	c.Check(pos, check.Equals, int64(0))
+	c.Check(err, check.IsNil)
+	f.Read(buf[:3])
+	pos, _ = f.Seek(0, os.SEEK_CUR)
+	c.Check(pos, check.Equals, int64(3))
+	f.Write([]byte{7, 8, 9})
+	pos, err = f.Seek(0, os.SEEK_CUR)
+	c.Check(pos, check.Equals, int64(9))
+	f.Close()
+
+	f, err = fs.OpenFile("wronly", os.O_CREATE|os.O_WRONLY, 0)
+	c.Assert(err, check.IsNil)
+	n, err = f.Write([]byte{3, 2, 1})
+	c.Check(n, check.Equals, 3)
+	c.Check(err, check.IsNil)
+	pos, _ = f.Seek(0, os.SEEK_CUR)
+	c.Check(pos, check.Equals, int64(3))
+	pos, _ = f.Seek(0, os.SEEK_SET)
+	c.Check(pos, check.Equals, int64(0))
+	n, err = f.Read(buf)
+	c.Check(n, check.Equals, 0)
+	c.Check(err, check.ErrorMatches, `.*O_WRONLY.*`)
+	f, err = fs.OpenFile("wronly", os.O_RDONLY, 0)
+	c.Assert(err, check.IsNil)
+	n, _ = f.Read(buf)
+	c.Check(buf[:n], check.DeepEquals, []byte{3, 2, 1})
+
+	f, err = fs.OpenFile("unsupported", os.O_CREATE|os.O_SYNC, 0)
+	c.Check(f, check.IsNil)
+	c.Check(err, check.NotNil)
+
+	f, err = fs.OpenFile("append", os.O_RDWR|os.O_WRONLY, 0)
+	c.Check(f, check.IsNil)
+	c.Check(err, check.ErrorMatches, `invalid flag.*`)
+}
+
 func (s *CollectionFSSuite) TestFlushFullBlocks(c *check.C) {
 	maxBlockSize = 1024
 	defer func() { maxBlockSize = 2 << 26 }()

commit 307188e3e1abfc9d1b1179d203454ffe8be36097
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Nov 15 13:24:16 2017 -0500

    12483: Support file create/write via webdav.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/client.go b/sdk/go/arvados/client.go
index 47a953a..a38d95c 100644
--- a/sdk/go/arvados/client.go
+++ b/sdk/go/arvados/client.go
@@ -5,6 +5,7 @@
 package arvados
 
 import (
+	"bytes"
 	"crypto/tls"
 	"encoding/json"
 	"fmt"
@@ -180,6 +181,10 @@ func anythingToValues(params interface{}) (url.Values, error) {
 //
 // path must not contain a query string.
 func (c *Client) RequestAndDecode(dst interface{}, method, path string, body io.Reader, params interface{}) error {
+	if body, ok := body.(io.Closer); ok {
+		// Ensure body is closed even if we error out early
+		defer body.Close()
+	}
 	urlString := c.apiURL(path)
 	urlValues, err := anythingToValues(params)
 	if err != nil {
@@ -202,6 +207,24 @@ func (c *Client) RequestAndDecode(dst interface{}, method, path string, body io.
 	return c.DoAndDecode(dst, req)
 }
 
+type resource interface {
+	resourceName() string
+}
+
+// UpdateBody returns an io.Reader suitable for use as an http.Request
+// Body for a create or update API call.
+func (c *Client) UpdateBody(rsc resource) io.Reader {
+	j, err := json.Marshal(rsc)
+	if err != nil {
+		// Return a reader that returns errors.
+		r, w := io.Pipe()
+		w.CloseWithError(err)
+		return r
+	}
+	v := url.Values{rsc.resourceName(): {string(j)}}
+	return bytes.NewBufferString(v.Encode())
+}
+
 func (c *Client) httpClient() *http.Client {
 	switch {
 	case c.Client != nil:
diff --git a/sdk/go/arvados/collection.go b/sdk/go/arvados/collection.go
index 61bcd7f..999b4e9 100644
--- a/sdk/go/arvados/collection.go
+++ b/sdk/go/arvados/collection.go
@@ -30,6 +30,10 @@ type Collection struct {
 	IsTrashed              bool       `json:"is_trashed,omitempty"`
 }
 
+func (c Collection) resourceName() string {
+	return "collection"
+}
+
 // SizedDigests returns the hash+size part of each data block
 // referenced by the collection.
 func (c *Collection) SizedDigests() ([]SizedDigest, error) {
diff --git a/services/keep-web/cache.go b/services/keep-web/cache.go
index ce1168a..9ee9990 100644
--- a/services/keep-web/cache.go
+++ b/services/keep-web/cache.go
@@ -86,6 +86,29 @@ func (c *cache) Stats() cacheStats {
 	}
 }
 
+// Update saves a modified version (fs) to an existing collection
+// (coll) and, if successful, updates the relevant cache entries so
+// subsequent calls to Get() reflect the modifications.
+func (c *cache) Update(client *arvados.Client, coll arvados.Collection, fs arvados.CollectionFileSystem) error {
+	c.setupOnce.Do(c.setup)
+
+	if m, err := fs.MarshalManifest("."); err != nil || m == coll.ManifestText {
+		return err
+	} else {
+		coll.ManifestText = m
+	}
+	var updated arvados.Collection
+	defer c.pdhs.Remove(coll.UUID)
+	err := client.RequestAndDecode(&updated, "PATCH", "/arvados/v1/collections/"+coll.UUID, client.UpdateBody(coll), nil)
+	if err == nil {
+		c.collections.Add(client.AuthToken+"\000"+coll.PortableDataHash, &cachedCollection{
+			expire:     time.Now().Add(time.Duration(c.TTL)),
+			collection: &updated,
+		})
+	}
+	return err
+}
+
 func (c *cache) Get(arv *arvadosclient.ArvadosClient, targetID string, forceReload bool) (*arvados.Collection, error) {
 	c.setupOnce.Do(c.setup)
 
diff --git a/services/keep-web/cadaver_test.go b/services/keep-web/cadaver_test.go
index 87a712f..eab4a70 100644
--- a/services/keep-web/cadaver_test.go
+++ b/services/keep-web/cadaver_test.go
@@ -7,42 +7,99 @@ package main
 import (
 	"bytes"
 	"io"
+	"io/ioutil"
+	"net/url"
+	"os"
 	"os/exec"
 
+	"git.curoverse.com/arvados.git/sdk/go/arvados"
 	"git.curoverse.com/arvados.git/sdk/go/arvadostest"
 	check "gopkg.in/check.v1"
 )
 
 func (s *IntegrationSuite) TestWebdavWithCadaver(c *check.C) {
-	basePath := "/c=" + arvadostest.FooAndBarFilesInDirUUID + "/t=" + arvadostest.ActiveToken + "/"
+	testdata := []byte("the human tragedy consists in the necessity of living with the consequences of actions performed under the pressure of compulsions we do not understand")
+
+	localfile, err := ioutil.TempFile("", "localfile")
+	c.Assert(err, check.IsNil)
+	defer os.Remove(localfile.Name())
+	localfile.Write(testdata)
+
+	emptyfile, err := ioutil.TempFile("", "emptyfile")
+	c.Assert(err, check.IsNil)
+	defer os.Remove(emptyfile.Name())
+
+	checkfile, err := ioutil.TempFile("", "checkfile")
+	c.Assert(err, check.IsNil)
+	defer os.Remove(checkfile.Name())
+
+	var newCollection arvados.Collection
+	arv := arvados.NewClientFromEnv()
+	arv.AuthToken = arvadostest.ActiveToken
+	err = arv.RequestAndDecode(&newCollection, "POST", "/arvados/v1/collections", bytes.NewBufferString(url.Values{"collection": {"{}"}}.Encode()), nil)
+	c.Assert(err, check.IsNil)
+	writePath := "/c=" + newCollection.UUID + "/t=" + arv.AuthToken + "/"
+
+	readPath := "/c=" + arvadostest.FooAndBarFilesInDirUUID + "/t=" + arvadostest.ActiveToken + "/"
 	type testcase struct {
 		path  string
 		cmd   string
 		match string
+		data  []byte
 	}
 	for _, trial := range []testcase{
 		{
-			path:  basePath,
+			path:  readPath,
 			cmd:   "ls\n",
 			match: `(?ms).*dir1 *0 .*`,
 		},
 		{
-			path:  basePath,
+			path:  readPath,
 			cmd:   "ls dir1\n",
 			match: `(?ms).*bar *3.*foo *3 .*`,
 		},
 		{
-			path:  basePath + "_/dir1",
+			path:  readPath + "_/dir1",
 			cmd:   "ls\n",
 			match: `(?ms).*bar *3.*foo *3 .*`,
 		},
 		{
-			path:  basePath + "dir1/",
+			path:  readPath + "dir1/",
 			cmd:   "ls\n",
 			match: `(?ms).*bar *3.*foo *3 .*`,
 		},
+		{
+			path:  writePath,
+			cmd:   "get emptyfile '" + checkfile.Name() + "'\n",
+			match: `(?ms).*Not Found.*`,
+		},
+		{
+			path:  writePath,
+			cmd:   "put '" + emptyfile.Name() + "' emptyfile\n",
+			match: `(?ms).*Uploading .* succeeded.*`,
+		},
+		{
+			path:  writePath,
+			cmd:   "get emptyfile '" + checkfile.Name() + "'\n",
+			match: `(?ms).*Downloading .* succeeded.*`,
+			data:  []byte{},
+		},
+		{
+			path:  writePath,
+			cmd:   "put '" + localfile.Name() + "' testfile\n",
+			match: `(?ms).*Uploading .* succeeded.*`,
+		},
+		{
+			path:  writePath,
+			cmd:   "get testfile '" + checkfile.Name() + "'\n",
+			match: `(?ms).*succeeded.*`,
+			data:  testdata,
+		},
 	} {
-		c.Logf("%s %#v", "http://"+s.testServer.Addr, trial)
+		c.Logf("%s %+v", "http://"+s.testServer.Addr, trial)
+
+		os.Remove(checkfile.Name())
+
 		cmd := exec.Command("cadaver", "http://"+s.testServer.Addr+trial.path)
 		cmd.Stdin = bytes.NewBufferString(trial.cmd)
 		stdout, err := cmd.StdoutPipe()
@@ -56,5 +113,15 @@ func (s *IntegrationSuite) TestWebdavWithCadaver(c *check.C) {
 		err = cmd.Wait()
 		c.Check(err, check.Equals, nil)
 		c.Check(buf.String(), check.Matches, trial.match)
+
+		if trial.data == nil {
+			continue
+		}
+		checkfile, err = os.Open(checkfile.Name())
+		c.Assert(err, check.IsNil)
+		checkfile.Seek(0, os.SEEK_SET)
+		got, err := ioutil.ReadAll(checkfile)
+		c.Check(got, check.DeepEquals, trial.data)
+		c.Check(err, check.IsNil)
 	}
 }
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index b4f3062..42f6c51 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -100,6 +100,7 @@ var (
 	webdavMethod = map[string]bool{
 		"OPTIONS":  true,
 		"PROPFIND": true,
+		"PUT":      true,
 	}
 	browserMethod = map[string]bool{
 		"GET":  true,
@@ -147,7 +148,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 			return
 		}
 		w.Header().Set("Access-Control-Allow-Headers", "Authorization, Content-Type, Range")
-		w.Header().Set("Access-Control-Allow-Methods", "GET, POST, OPTIONS, PROPFIND")
+		w.Header().Set("Access-Control-Allow-Methods", "GET, POST, OPTIONS, PROPFIND, PUT")
 		w.Header().Set("Access-Control-Allow-Origin", "*")
 		w.Header().Set("Access-Control-Max-Age", "86400")
 		statusCode = http.StatusOK
@@ -352,19 +353,29 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	}
 	applyContentDispositionHdr(w, r, basename, attachment)
 
-	fs, err := collection.FileSystem(&arvados.Client{
+	client := &arvados.Client{
 		APIHost:   arv.ApiServer,
 		AuthToken: arv.ApiToken,
 		Insecure:  arv.ApiInsecure,
-	}, kc)
+	}
+	fs, err := collection.FileSystem(client, kc)
 	if err != nil {
 		statusCode, statusText = http.StatusInternalServerError, err.Error()
 		return
 	}
 	if webdavMethod[r.Method] {
+		var update func() error
+		if !arvadosclient.PDHMatch(targetID) {
+			update = func() error {
+				return h.Config.Cache.Update(client, *collection, fs)
+			}
+		}
 		h := webdav.Handler{
-			Prefix:     "/" + strings.Join(pathParts[:stripParts], "/"),
-			FileSystem: &webdavFS{collfs: fs},
+			Prefix: "/" + strings.Join(pathParts[:stripParts], "/"),
+			FileSystem: &webdavFS{
+				collfs: fs,
+				update: update,
+			},
 			LockSystem: h.webdavLS,
 			Logger: func(_ *http.Request, err error) {
 				if os.IsNotExist(err) {
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 6bd34d7..32174cb 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -45,7 +45,7 @@ func (s *UnitSuite) TestCORSPreflight(c *check.C) {
 	c.Check(resp.Code, check.Equals, http.StatusOK)
 	c.Check(resp.Body.String(), check.Equals, "")
 	c.Check(resp.Header().Get("Access-Control-Allow-Origin"), check.Equals, "*")
-	c.Check(resp.Header().Get("Access-Control-Allow-Methods"), check.Equals, "GET, POST, OPTIONS, PROPFIND")
+	c.Check(resp.Header().Get("Access-Control-Allow-Methods"), check.Equals, "GET, POST, OPTIONS, PROPFIND, PUT")
 	c.Check(resp.Header().Get("Access-Control-Allow-Headers"), check.Equals, "Authorization, Content-Type, Range")
 
 	// Check preflight for a disallowed request
diff --git a/services/keep-web/webdav.go b/services/keep-web/webdav.go
index 57f3f53..87d9c47 100644
--- a/services/keep-web/webdav.go
+++ b/services/keep-web/webdav.go
@@ -9,9 +9,7 @@ import (
 	"errors"
 	"fmt"
 	prand "math/rand"
-	"net/http"
 	"os"
-	"sync"
 	"sync/atomic"
 	"time"
 
@@ -27,24 +25,27 @@ var (
 	errReadOnly           = errors.New("read-only filesystem")
 )
 
-// webdavFS implements a read-only webdav.FileSystem by wrapping an
+// webdavFS implements a webdav.FileSystem by wrapping an
 // arvados.CollectionFilesystem.
 type webdavFS struct {
 	collfs arvados.CollectionFileSystem
+	update func() error
 }
 
-var _ webdav.FileSystem = &webdavFS{}
-
 func (fs *webdavFS) Mkdir(ctx context.Context, name string, perm os.FileMode) error {
-	return errReadOnly
+	return fs.collfs.Mkdir(name, 0755)
 }
 
-func (fs *webdavFS) OpenFile(ctx context.Context, name string, flag int, perm os.FileMode) (webdav.File, error) {
-	fi, err := fs.collfs.Stat(name)
-	if err != nil {
-		return nil, err
+func (fs *webdavFS) OpenFile(ctx context.Context, name string, flag int, perm os.FileMode) (f webdav.File, err error) {
+	writing := flag&(os.O_WRONLY|os.O_RDWR) != 0
+	if writing && fs.update == nil {
+		return nil, errReadOnly
+	}
+	f, err = fs.collfs.OpenFile(name, flag, perm)
+	if writing && err == nil {
+		f = writingFile{File: f, update: fs.update}
 	}
-	return &webdavFile{collfs: fs.collfs, fileInfo: fi, name: name}, nil
+	return
 }
 
 func (fs *webdavFS) RemoveAll(ctx context.Context, name string) error {
@@ -59,71 +60,16 @@ func (fs *webdavFS) Stat(ctx context.Context, name string) (os.FileInfo, error)
 	return fs.collfs.Stat(name)
 }
 
-// webdavFile implements a read-only webdav.File by wrapping
-// http.File.
-//
-// The http.File is opened from an arvados.CollectionFileSystem, but
-// not until Seek, Read, or Readdir is called. This deferred-open
-// strategy makes webdav's OpenFile-Stat-Close cycle fast even though
-// the collfs's Open method is slow. This is relevant because webdav
-// does OpenFile-Stat-Close on each file when preparing directory
-// listings.
-//
-// Writes to a webdavFile always fail.
-type webdavFile struct {
-	// fields populated by (*webdavFS).OpenFile()
-	collfs   http.FileSystem
-	fileInfo os.FileInfo
-	name     string
-
-	// internal fields
-	file     http.File
-	loadOnce sync.Once
-	err      error
-}
-
-func (f *webdavFile) load() {
-	f.file, f.err = f.collfs.Open(f.name)
-}
-
-func (f *webdavFile) Write([]byte) (int, error) {
-	return 0, errReadOnly
-}
-
-func (f *webdavFile) Seek(offset int64, whence int) (int64, error) {
-	f.loadOnce.Do(f.load)
-	if f.err != nil {
-		return 0, f.err
-	}
-	return f.file.Seek(offset, whence)
-}
-
-func (f *webdavFile) Read(buf []byte) (int, error) {
-	f.loadOnce.Do(f.load)
-	if f.err != nil {
-		return 0, f.err
-	}
-	return f.file.Read(buf)
-}
-
-func (f *webdavFile) Close() error {
-	if f.file == nil {
-		// We never called load(), or load() failed
-		return f.err
-	}
-	return f.file.Close()
+type writingFile struct {
+	webdav.File
+	update func() error
 }
 
-func (f *webdavFile) Readdir(n int) ([]os.FileInfo, error) {
-	f.loadOnce.Do(f.load)
-	if f.err != nil {
-		return nil, f.err
+func (f writingFile) Close() error {
+	if err := f.File.Close(); err != nil || f.update == nil {
+		return err
 	}
-	return f.file.Readdir(n)
-}
-
-func (f *webdavFile) Stat() (os.FileInfo, error) {
-	return f.fileInfo, nil
+	return f.update()
 }
 
 // noLockSystem implements webdav.LockSystem by returning success for
diff --git a/services/keep-web/webdav_test.go b/services/keep-web/webdav_test.go
new file mode 100644
index 0000000..52db776
--- /dev/null
+++ b/services/keep-web/webdav_test.go
@@ -0,0 +1,5 @@
+package main
+
+import "golang.org/x/net/webdav"
+
+var _ webdav.FileSystem = &webdavFS{}

commit b33889df9191d3725212220c1304fdeb9c9798a9
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Nov 15 13:14:13 2017 -0500

    12483: Fix makeParentDirs bug when higher levels already exist.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 6019cfb..328d68f 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -794,7 +794,10 @@ func (dn *dirnode) loadManifest(txt string) error {
 				return fmt.Errorf("line %d: bad file segment %q", lineno, token)
 			}
 			name := path.Clean(dirname + "/" + manifestUnescape(toks[2]))
-			dn.makeParentDirs(name)
+			err = dn.makeParentDirs(name)
+			if err != nil {
+				return fmt.Errorf("line %d: cannot use path %q: %s", lineno, name, err)
+			}
 			f, err := dn.OpenFile(name, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0700)
 			if err != nil {
 				return fmt.Errorf("line %d: cannot append to %q: %s", lineno, name, err)
@@ -852,7 +855,7 @@ func (dn *dirnode) loadManifest(txt string) error {
 func (dn *dirnode) makeParentDirs(name string) (err error) {
 	names := strings.Split(name, "/")
 	for _, name := range names[:len(names)-1] {
-		f, err := dn.mkdir(name)
+		f, err := dn.OpenFile(name, os.O_CREATE, os.ModeDir|0755)
 		if err != nil {
 			return err
 		}
@@ -872,8 +875,8 @@ func (dn *dirnode) mkdir(name string) (*file, error) {
 
 func (dn *dirnode) Mkdir(name string, perm os.FileMode) error {
 	f, err := dn.mkdir(name)
-	if err != nil {
-		f.Close()
+	if err == nil {
+		err = f.Close()
 	}
 	return err
 }
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 6dc7286..ecb1f89 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -578,19 +578,20 @@ func (s *CollectionFSSuite) TestPersistEmptyFiles(c *check.C) {
 	var err error
 	s.fs, err = (&Collection{}).FileSystem(s.client, s.kc)
 	c.Assert(err, check.IsNil)
-	for _, name := range []string{"dir", "zero", "zero/zero"} {
+	for _, name := range []string{"dir", "dir/zerodir", "zero", "zero/zero"} {
 		err = s.fs.Mkdir(name, 0755)
 		c.Assert(err, check.IsNil)
 	}
 
 	expect := map[string][]byte{
-		"0":              nil,
-		"00":             []byte{},
-		"one":            []byte{1},
-		"dir/0":          nil,
-		"dir/two":        []byte{1, 2},
-		"dir/zero":       nil,
-		"zero/zero/zero": nil,
+		"0":                nil,
+		"00":               []byte{},
+		"one":              []byte{1},
+		"dir/0":            nil,
+		"dir/two":          []byte{1, 2},
+		"dir/zero":         nil,
+		"dir/zerodir/zero": nil,
+		"zero/zero/zero":   nil,
 	}
 	for name, data := range expect {
 		f, err := s.fs.OpenFile(name, os.O_WRONLY|os.O_CREATE, 0)

commit 794d493c18ec19b7c2437eb898b5e13645e6ca2a
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Nov 15 13:01:29 2017 -0500

    12483: Persist empty files.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 7962650..6019cfb 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -699,6 +699,10 @@ func (dn *dirnode) marshalManifest(prefix string) (string, error) {
 			}
 			subdirs = subdirs + subdir
 		case *filenode:
+			if len(node.extents) == 0 {
+				segments = append(segments, m1segment{name: name})
+				break
+			}
 			for _, e := range node.extents {
 				switch e := e.(type) {
 				case storedExtent:
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 4be1a3d..6dc7286 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -574,6 +574,57 @@ func (s *CollectionFSSuite) TestPersist(c *check.C) {
 	}
 }
 
+func (s *CollectionFSSuite) TestPersistEmptyFiles(c *check.C) {
+	var err error
+	s.fs, err = (&Collection{}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
+	for _, name := range []string{"dir", "zero", "zero/zero"} {
+		err = s.fs.Mkdir(name, 0755)
+		c.Assert(err, check.IsNil)
+	}
+
+	expect := map[string][]byte{
+		"0":              nil,
+		"00":             []byte{},
+		"one":            []byte{1},
+		"dir/0":          nil,
+		"dir/two":        []byte{1, 2},
+		"dir/zero":       nil,
+		"zero/zero/zero": nil,
+	}
+	for name, data := range expect {
+		f, err := s.fs.OpenFile(name, os.O_WRONLY|os.O_CREATE, 0)
+		c.Assert(err, check.IsNil)
+		if data != nil {
+			_, err := f.Write(data)
+			c.Assert(err, check.IsNil)
+		}
+		f.Close()
+	}
+
+	m, err := s.fs.MarshalManifest(".")
+	c.Check(err, check.IsNil)
+	c.Logf("%q", m)
+
+	persisted, err := (&Collection{ManifestText: m}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
+
+	for name, data := range expect {
+		f, err := persisted.Open("bogus-" + name)
+		c.Check(err, check.NotNil)
+
+		f, err = persisted.Open(name)
+		c.Assert(err, check.IsNil)
+
+		if data == nil {
+			data = []byte{}
+		}
+		buf, err := ioutil.ReadAll(f)
+		c.Check(err, check.IsNil)
+		c.Check(buf, check.DeepEquals, data)
+	}
+}
+
 func (s *CollectionFSSuite) TestFlushFullBlocks(c *check.C) {
 	maxBlockSize = 1024
 	defer func() { maxBlockSize = 2 << 26 }()

commit 056a3fa0fcace48ea0053ed5b89cd6e0d1ca792e
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Nov 14 15:17:12 2017 -0500

    12483: Track memory use. Flush filled blocks while writing.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 0499a04..7962650 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -139,6 +139,7 @@ type filenode struct {
 	parent   *dirnode
 	extents  []extent
 	repacked int64 // number of times anything in []extents has changed len
+	memsize  int64 // bytes in memExtents
 	sync.RWMutex
 }
 
@@ -273,15 +274,21 @@ func (fn *filenode) Truncate(size int64) error {
 	defer fn.Unlock()
 	if size < fn.fileinfo.size {
 		ptr := fn.seek(filenodePtr{off: size, repacked: fn.repacked - 1})
+		for i := ptr.extentIdx; i < len(fn.extents); i++ {
+			if ext, ok := fn.extents[i].(*memExtent); ok {
+				fn.memsize -= int64(ext.Len())
+			}
+		}
 		if ptr.extentOff == 0 {
 			fn.extents = fn.extents[:ptr.extentIdx]
 		} else {
 			fn.extents = fn.extents[:ptr.extentIdx+1]
-			e := fn.extents[ptr.extentIdx]
-			if e, ok := e.(writableExtent); ok {
-				e.Truncate(ptr.extentOff)
-			} else {
-				fn.extents[ptr.extentIdx] = e.Slice(0, ptr.extentOff)
+			switch ext := fn.extents[ptr.extentIdx].(type) {
+			case *memExtent:
+				ext.Truncate(ptr.extentOff)
+				fn.memsize += int64(ext.Len())
+			default:
+				fn.extents[ptr.extentIdx] = ext.Slice(0, ptr.extentOff)
 			}
 		}
 		fn.fileinfo.size = size
@@ -306,6 +313,7 @@ func (fn *filenode) Truncate(size int64) error {
 		}
 		e.Truncate(e.Len() + int(grow))
 		fn.fileinfo.size += grow
+		fn.memsize += grow
 	}
 	return nil
 }
@@ -357,6 +365,7 @@ func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePt
 			prev++
 			e := &memExtent{}
 			e.Truncate(len(cando))
+			fn.memsize += int64(len(cando))
 			fn.extents[cur] = e
 			fn.extents[prev] = fn.extents[prev].Slice(0, ptr.extentOff)
 			ptr.extentIdx++
@@ -398,6 +407,7 @@ func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePt
 				ptr.extentIdx--
 				ptr.extentOff = fn.extents[prev].Len()
 				fn.extents[prev].(writableExtent).Truncate(ptr.extentOff + len(cando))
+				fn.memsize += int64(len(cando))
 				ptr.repacked++
 				fn.repacked++
 			} else {
@@ -413,6 +423,7 @@ func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePt
 				}
 				e := &memExtent{}
 				e.Truncate(len(cando))
+				fn.memsize += int64(len(cando))
 				fn.extents[cur] = e
 				cur++
 				prev++
@@ -426,6 +437,9 @@ func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePt
 
 		ptr.off += int64(len(cando))
 		ptr.extentOff += len(cando)
+		if ptr.extentOff >= maxBlockSize {
+			fn.pruneMemExtents()
+		}
 		if fn.extents[ptr.extentIdx].Len() == ptr.extentOff {
 			ptr.extentOff = 0
 			ptr.extentIdx++
@@ -434,6 +448,35 @@ func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePt
 	return
 }
 
+// Write some data out to disk to reduce memory use. Caller must have
+// write lock.
+func (fn *filenode) pruneMemExtents() {
+	// TODO: async (don't hold Lock() while waiting for Keep)
+	// TODO: share code with (*dirnode)sync()
+	// TODO: pack/flush small blocks too, when fragmented
+	for idx, ext := range fn.extents {
+		ext, ok := ext.(*memExtent)
+		if !ok || ext.Len() < maxBlockSize {
+			continue
+		}
+		locator, _, err := fn.parent.kc.PutB(ext.buf)
+		if err != nil {
+			// TODO: stall (or return errors from)
+			// subsequent writes until flushing
+			// starts to succeed
+			continue
+		}
+		fn.memsize -= int64(ext.Len())
+		fn.extents[idx] = storedExtent{
+			kc:      fn.parent.kc,
+			locator: locator,
+			size:    ext.Len(),
+			offset:  0,
+			length:  ext.Len(),
+		}
+	}
+}
+
 // FileSystem returns a CollectionFileSystem for the collection.
 func (c *Collection) FileSystem(client *Client, kc keepClient) (CollectionFileSystem, error) {
 	fs := &fileSystem{dirnode: dirnode{
@@ -540,7 +583,8 @@ type dirnode struct {
 	sync.RWMutex
 }
 
-// caller must hold dn.Lock().
+// sync flushes in-memory data (for all files in the tree rooted at
+// dn) to persistent storage. Caller must hold dn.Lock().
 func (dn *dirnode) sync() error {
 	type shortBlock struct {
 		fn  *filenode
@@ -572,6 +616,7 @@ func (dn *dirnode) sync() error {
 				length:  len(data),
 			}
 			off += len(data)
+			sb.fn.memsize -= int64(len(data))
 		}
 		return nil
 	}
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 4e2e84f..4be1a3d 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -307,6 +307,11 @@ func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
 	c.Check(string(buf2), check.Equals, "12345678abcdefg")
 	c.Check(len(f.(*file).inode.(*filenode).extents), check.Equals, 2)
 
+	// Force flush to ensure the block "12345678" gets stored, so
+	// we know what to expect in the final manifest below.
+	_, err = s.fs.MarshalManifest(".")
+	c.Check(err, check.IsNil)
+
 	// Truncate to size=3 while f2's ptr is at 15
 	err = f.Truncate(3)
 	c.Check(err, check.IsNil)
@@ -322,7 +327,7 @@ func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
 	m, err := s.fs.MarshalManifest(".")
 	c.Check(err, check.IsNil)
 	m = regexp.MustCompile(`\+A[^\+ ]+`).ReplaceAllLiteralString(m, "")
-	c.Check(m, check.Equals, "./dir1 3858f62230ac3c915f300c664312c63f+6 202cb962ac59075b964b07152d234b70+3 3:3:bar 6:3:foo\n")
+	c.Check(m, check.Equals, "./dir1 3858f62230ac3c915f300c664312c63f+6 25d55ad283aa400af464c76d713c07ad+8 3:3:bar 6:3:foo\n")
 }
 
 func (s *CollectionFSSuite) TestMarshalSmallBlocks(c *check.C) {
@@ -439,15 +444,18 @@ func (s *CollectionFSSuite) TestConcurrentWriters(c *check.C) {
 }
 
 func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
-	maxBlockSize = 8
+	maxBlockSize = 40
 	defer func() { maxBlockSize = 2 << 26 }()
 
 	var err error
 	s.fs, err = (&Collection{}).FileSystem(s.client, s.kc)
 	c.Assert(err, check.IsNil)
 
+	const nfiles = 256
+	const ngoroutines = 256
+
 	var wg sync.WaitGroup
-	for n := 0; n < 128; n++ {
+	for n := 0; n < nfiles; n++ {
 		wg.Add(1)
 		go func(n int) {
 			defer wg.Done()
@@ -456,7 +464,7 @@ func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
 			f, err := s.fs.OpenFile(fmt.Sprintf("random-%d", n), os.O_RDWR|os.O_CREATE|os.O_EXCL, 0)
 			c.Assert(err, check.IsNil)
 			defer f.Close()
-			for i := 0; i < 6502; i++ {
+			for i := 0; i < ngoroutines; i++ {
 				trunc := rand.Intn(65)
 				woff := rand.Intn(trunc + 1)
 				wbytes = wbytes[:rand.Intn(64-woff+1)]
@@ -482,6 +490,7 @@ func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
 				c.Check(string(buf), check.Equals, string(expect))
 				c.Check(err, check.IsNil)
 			}
+			s.checkMemSize(c, f)
 		}(n)
 	}
 	wg.Wait()
@@ -491,7 +500,7 @@ func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
 	defer root.Close()
 	fi, err := root.Readdir(-1)
 	c.Check(err, check.IsNil)
-	c.Check(len(fi), check.Equals, 128)
+	c.Check(len(fi), check.Equals, nfiles)
 
 	_, err = s.fs.MarshalManifest(".")
 	c.Check(err, check.IsNil)
@@ -565,6 +574,42 @@ func (s *CollectionFSSuite) TestPersist(c *check.C) {
 	}
 }
 
+func (s *CollectionFSSuite) TestFlushFullBlocks(c *check.C) {
+	maxBlockSize = 1024
+	defer func() { maxBlockSize = 2 << 26 }()
+
+	fs, err := (&Collection{}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
+	f, err := fs.OpenFile("50K", os.O_WRONLY|os.O_CREATE, 0)
+	c.Assert(err, check.IsNil)
+	defer f.Close()
+
+	data := make([]byte, 500)
+	rand.Read(data)
+
+	for i := 0; i < 100; i++ {
+		n, err := f.Write(data)
+		c.Assert(n, check.Equals, len(data))
+		c.Assert(err, check.IsNil)
+	}
+
+	currentMemExtents := func() (memExtents []int) {
+		for idx, e := range f.(*file).inode.(*filenode).extents {
+			switch e.(type) {
+			case *memExtent:
+				memExtents = append(memExtents, idx)
+			}
+		}
+		return
+	}
+	c.Check(currentMemExtents(), check.HasLen, 1)
+
+	m, err := fs.MarshalManifest(".")
+	c.Check(m, check.Not(check.Equals), "")
+	c.Check(err, check.IsNil)
+	c.Check(currentMemExtents(), check.HasLen, 0)
+}
+
 func (s *CollectionFSSuite) TestBrokenManifests(c *check.C) {
 	for _, txt := range []string{
 		"\n",
@@ -606,6 +651,17 @@ func (s *CollectionFSSuite) TestEdgeCaseManifests(c *check.C) {
 	}
 }
 
+func (s *CollectionFSSuite) checkMemSize(c *check.C, f File) {
+	fn := f.(*file).inode.(*filenode)
+	var memsize int64
+	for _, ext := range fn.extents {
+		if e, ok := ext.(*memExtent); ok {
+			memsize += int64(len(e.buf))
+		}
+	}
+	c.Check(fn.memsize, check.Equals, memsize)
+}
+
 // Gocheck boilerplate
 func Test(t *testing.T) {
 	check.TestingT(t)

commit 375602be6ccd6d2293af6b3af7dbff005e782e29
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Nov 14 09:34:13 2017 -0500

    12483: Fix missing Rlock() on inode accesses.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 1214de4..0499a04 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -83,10 +83,6 @@ func (fi fileinfo) Sys() interface{} {
 	return nil
 }
 
-func (fi fileinfo) Stat() os.FileInfo {
-	return fi
-}
-
 // A CollectionFileSystem is an http.Filesystem plus Stat() and
 // support for opening writable files.
 type CollectionFileSystem interface {
@@ -125,12 +121,12 @@ func (fs *fileSystem) Stat(name string) (os.FileInfo, error) {
 }
 
 type inode interface {
-	os.FileInfo
 	Parent() inode
 	Read([]byte, filenodePtr) (int, filenodePtr, error)
 	Write([]byte, filenodePtr) (int, filenodePtr, error)
 	Truncate(int64) error
 	Readdir() []os.FileInfo
+	Size() int64
 	Stat() os.FileInfo
 	sync.Locker
 	RLock()
@@ -139,7 +135,7 @@ type inode interface {
 
 // filenode implements inode.
 type filenode struct {
-	fileinfo
+	fileinfo fileinfo
 	parent   *dirnode
 	extents  []extent
 	repacked int64 // number of times anything in []extents has changed len
@@ -260,6 +256,18 @@ func (fn *filenode) Read(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr
 	return
 }
 
+func (fn *filenode) Size() int64 {
+	fn.RLock()
+	defer fn.RUnlock()
+	return fn.fileinfo.Size()
+}
+
+func (fn *filenode) Stat() os.FileInfo {
+	fn.RLock()
+	defer fn.RUnlock()
+	return fn.fileinfo
+}
+
 func (fn *filenode) Truncate(size int64) error {
 	fn.Lock()
 	defer fn.Unlock()
@@ -494,7 +502,7 @@ func (f *file) Write(p []byte) (n int, err error) {
 }
 
 func (f *file) Readdir(count int) ([]os.FileInfo, error) {
-	if !f.inode.IsDir() {
+	if !f.inode.Stat().IsDir() {
 		return nil, ErrInvalidOperation
 	}
 	if count <= 0 {
@@ -515,7 +523,7 @@ func (f *file) Readdir(count int) ([]os.FileInfo, error) {
 }
 
 func (f *file) Stat() (os.FileInfo, error) {
-	return f.inode, nil
+	return f.inode.Stat(), nil
 }
 
 func (f *file) Close() error {
@@ -524,11 +532,11 @@ func (f *file) Close() error {
 }
 
 type dirnode struct {
-	fileinfo
-	parent *dirnode
-	client *Client
-	kc     keepClient
-	inodes map[string]inode
+	fileinfo fileinfo
+	parent   *dirnode
+	client   *Client
+	kc       keepClient
+	inodes   map[string]inode
 	sync.RWMutex
 }
 
@@ -640,7 +648,7 @@ func (dn *dirnode) marshalManifest(prefix string) (string, error) {
 		node := dn.inodes[name]
 		switch node := node.(type) {
 		case *dirnode:
-			subdir, err := node.marshalManifest(prefix + "/" + node.Name())
+			subdir, err := node.marshalManifest(prefix + "/" + name)
 			if err != nil {
 				return "", err
 			}
@@ -655,7 +663,7 @@ func (dn *dirnode) marshalManifest(prefix string) (string, error) {
 						blocks = append(blocks, e.locator)
 					}
 					segments = append(segments, m1segment{
-						name:   node.Name(),
+						name:   name,
 						offset: streamLen + int64(e.offset),
 						length: int64(e.length),
 					})
@@ -870,6 +878,18 @@ func (dn *dirnode) Write(p []byte, ptr filenodePtr) (int, filenodePtr, error) {
 	return 0, ptr, ErrInvalidOperation
 }
 
+func (dn *dirnode) Size() int64 {
+	dn.RLock()
+	defer dn.RUnlock()
+	return dn.fileinfo.Size()
+}
+
+func (dn *dirnode) Stat() os.FileInfo {
+	dn.RLock()
+	defer dn.RUnlock()
+	return dn.fileinfo
+}
+
 func (dn *dirnode) Truncate(int64) error {
 	return ErrInvalidOperation
 }

commit 1ee5740919bbfa0075d9e7d1afd44fdeaef54b03
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Nov 14 00:55:58 2017 -0500

    12483: Return error on impossible manifest.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index eb92f16..1214de4 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -427,7 +427,7 @@ func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePt
 }
 
 // FileSystem returns a CollectionFileSystem for the collection.
-func (c *Collection) FileSystem(client *Client, kc keepClient) CollectionFileSystem {
+func (c *Collection) FileSystem(client *Client, kc keepClient) (CollectionFileSystem, error) {
 	fs := &fileSystem{dirnode: dirnode{
 		client:   client,
 		kc:       kc,
@@ -436,8 +436,10 @@ func (c *Collection) FileSystem(client *Client, kc keepClient) CollectionFileSys
 		inodes:   make(map[string]inode),
 	}}
 	fs.dirnode.parent = &fs.dirnode
-	fs.dirnode.loadManifest(c.ManifestText)
-	return fs
+	if err := fs.dirnode.loadManifest(c.ManifestText); err != nil {
+		return nil, err
+	}
+	return fs, nil
 }
 
 type file struct {
@@ -681,26 +683,33 @@ func (dn *dirnode) marshalManifest(prefix string) (string, error) {
 	return manifestEscape(prefix) + " " + strings.Join(blocks, " ") + " " + strings.Join(filetokens, " ") + "\n" + subdirs, nil
 }
 
-func (dn *dirnode) loadManifest(txt string) {
+func (dn *dirnode) loadManifest(txt string) error {
 	// FIXME: faster
 	var dirname string
-	for _, stream := range strings.Split(txt, "\n") {
+	streams := strings.Split(txt, "\n")
+	if streams[len(streams)-1] != "" {
+		return fmt.Errorf("line %d: no trailing newline", len(streams))
+	}
+	for i, stream := range streams[:len(streams)-1] {
+		lineno := i + 1
 		var extents []storedExtent
+		var anyFileTokens bool
 		for i, token := range strings.Split(stream, " ") {
 			if i == 0 {
 				dirname = manifestUnescape(token)
 				continue
 			}
 			if !strings.Contains(token, ":") {
+				if anyFileTokens {
+					return fmt.Errorf("line %d: bad file segment %q", lineno, token)
+				}
 				toks := strings.SplitN(token, "+", 3)
 				if len(toks) < 2 {
-					// FIXME: broken
-					continue
+					return fmt.Errorf("line %d: bad locator %q", lineno, token)
 				}
 				length, err := strconv.ParseInt(toks[1], 10, 32)
 				if err != nil || length < 0 {
-					// FIXME: broken
-					continue
+					return fmt.Errorf("line %d: bad locator %q", lineno, token)
 				}
 				extents = append(extents, storedExtent{
 					locator: token,
@@ -709,33 +718,33 @@ func (dn *dirnode) loadManifest(txt string) {
 					length:  int(length),
 				})
 				continue
+			} else if len(extents) == 0 {
+				return fmt.Errorf("line %d: bad locator %q", lineno, token)
 			}
+
 			toks := strings.Split(token, ":")
 			if len(toks) != 3 {
-				// FIXME: broken manifest
-				continue
+				return fmt.Errorf("line %d: bad file segment %q", lineno, token)
 			}
+			anyFileTokens = true
+
 			offset, err := strconv.ParseInt(toks[0], 10, 64)
 			if err != nil || offset < 0 {
-				// FIXME: broken manifest
-				continue
+				return fmt.Errorf("line %d: bad file segment %q", lineno, token)
 			}
 			length, err := strconv.ParseInt(toks[1], 10, 64)
 			if err != nil || length < 0 {
-				// FIXME: broken manifest
-				continue
+				return fmt.Errorf("line %d: bad file segment %q", lineno, token)
 			}
 			name := path.Clean(dirname + "/" + manifestUnescape(toks[2]))
 			dn.makeParentDirs(name)
 			f, err := dn.OpenFile(name, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0700)
 			if err != nil {
-				// FIXME: don't panic
-				panic(fmt.Errorf("cannot append to %q: %s", name, err))
+				return fmt.Errorf("line %d: cannot append to %q: %s", lineno, name, err)
 			}
 			if f.inode.Stat().IsDir() {
 				f.Close()
-				// FIXME: don't panic
-				panic(fmt.Errorf("cannot append to %q: is a directory", name))
+				return fmt.Errorf("line %d: cannot append to %q: is a directory", lineno, name)
 			}
 			// Map the stream offset/range coordinates to
 			// block/offset/range coordinates and add
@@ -768,8 +777,19 @@ func (dn *dirnode) loadManifest(txt string) {
 				pos = next
 			}
 			f.Close()
+			if pos < offset+length {
+				return fmt.Errorf("line %d: invalid segment in %d-byte stream: %q", lineno, pos, token)
+			}
+		}
+		if !anyFileTokens {
+			return fmt.Errorf("line %d: no file segments", lineno)
+		} else if len(extents) == 0 {
+			return fmt.Errorf("line %d: no locators", lineno)
+		} else if dirname == "" {
+			return fmt.Errorf("line %d: no stream name", lineno)
 		}
 	}
+	return nil
 }
 
 func (dn *dirnode) makeParentDirs(name string) (err error) {
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 5d49ba3..4e2e84f 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -65,7 +65,8 @@ func (s *CollectionFSSuite) SetUpTest(c *check.C) {
 		blocks: map[string][]byte{
 			"3858f62230ac3c915f300c664312c63f": []byte("foobar"),
 		}}
-	s.fs = s.coll.FileSystem(s.client, s.kc)
+	s.fs, err = s.coll.FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
 }
 
 func (s *CollectionFSSuite) TestHttpFileSystemInterface(c *check.C) {
@@ -328,7 +329,9 @@ func (s *CollectionFSSuite) TestMarshalSmallBlocks(c *check.C) {
 	maxBlockSize = 8
 	defer func() { maxBlockSize = 2 << 26 }()
 
-	s.fs = (&Collection{}).FileSystem(s.client, s.kc)
+	var err error
+	s.fs, err = (&Collection{}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
 	for _, name := range []string{"foo", "bar", "baz"} {
 		f, err := s.fs.OpenFile(name, os.O_WRONLY|os.O_CREATE, 0)
 		c.Assert(err, check.IsNil)
@@ -439,7 +442,9 @@ func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
 	maxBlockSize = 8
 	defer func() { maxBlockSize = 2 << 26 }()
 
-	s.fs = (&Collection{}).FileSystem(s.client, s.kc)
+	var err error
+	s.fs, err = (&Collection{}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
 
 	var wg sync.WaitGroup
 	for n := 0; n < 128; n++ {
@@ -497,8 +502,10 @@ func (s *CollectionFSSuite) TestPersist(c *check.C) {
 	maxBlockSize = 1024
 	defer func() { maxBlockSize = 2 << 26 }()
 
-	s.fs = (&Collection{}).FileSystem(s.client, s.kc)
-	err := s.fs.Mkdir("d:r", 0755)
+	var err error
+	s.fs, err = (&Collection{}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
+	err = s.fs.Mkdir("d:r", 0755)
 	c.Assert(err, check.IsNil)
 
 	expect := map[string][]byte{}
@@ -537,7 +544,8 @@ func (s *CollectionFSSuite) TestPersist(c *check.C) {
 	c.Check(err, check.IsNil)
 	c.Check(len(fi), check.Equals, 4)
 
-	persisted := (&Collection{ManifestText: m}).FileSystem(s.client, s.kc)
+	persisted, err := (&Collection{ManifestText: m}).FileSystem(s.client, s.kc)
+	c.Assert(err, check.IsNil)
 
 	root, err = persisted.Open("/")
 	c.Assert(err, check.IsNil)
@@ -557,6 +565,47 @@ func (s *CollectionFSSuite) TestPersist(c *check.C) {
 	}
 }
 
+func (s *CollectionFSSuite) TestBrokenManifests(c *check.C) {
+	for _, txt := range []string{
+		"\n",
+		".\n",
+		". \n",
+		". d41d8cd98f00b204e9800998ecf8427e+0\n",
+		". d41d8cd98f00b204e9800998ecf8427e+0 \n",
+		". 0:0:foo\n",
+		".  0:0:foo\n",
+		". 0:0:foo 0:0:bar\n",
+		". d41d8cd98f00b204e9800998ecf8427e 0:0:foo\n",
+		". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo:bar\n",
+		". d41d8cd98f00b204e9800998ecf8427e+0 foo:0:foo\n",
+		". d41d8cd98f00b204e9800998ecf8427e+0 0:foo:foo\n",
+		". d41d8cd98f00b204e9800998ecf8427e+1 0:1:foo 1:1:bar\n",
+		". d41d8cd98f00b204e9800998ecf8427e+1 0:0:foo\n./foo d41d8cd98f00b204e9800998ecf8427e+1 0:0:bar\n",
+		"./foo d41d8cd98f00b204e9800998ecf8427e+1 0:0:bar\n. d41d8cd98f00b204e9800998ecf8427e+1 0:0:foo\n",
+	} {
+		c.Logf("<-%q", txt)
+		fs, err := (&Collection{ManifestText: txt}).FileSystem(s.client, s.kc)
+		c.Check(fs, check.IsNil)
+		c.Logf("-> %s", err)
+		c.Check(err, check.NotNil)
+	}
+}
+
+func (s *CollectionFSSuite) TestEdgeCaseManifests(c *check.C) {
+	for _, txt := range []string{
+		"",
+		". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n",
+		". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo 0:0:foo 0:0:bar\n",
+		". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo 0:0:foo 0:0:bar\n",
+		". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo/bar\n./foo d41d8cd98f00b204e9800998ecf8427e+0 0:0:bar\n",
+	} {
+		c.Logf("<-%q", txt)
+		fs, err := (&Collection{ManifestText: txt}).FileSystem(s.client, s.kc)
+		c.Check(err, check.IsNil)
+		c.Check(fs, check.NotNil)
+	}
+}
+
 // Gocheck boilerplate
 func Test(t *testing.T) {
 	check.TestingT(t)
diff --git a/sdk/go/keepclient/collectionreader.go b/sdk/go/keepclient/collectionreader.go
index 3f39aff..c071d3b 100644
--- a/sdk/go/keepclient/collectionreader.go
+++ b/sdk/go/keepclient/collectionreader.go
@@ -43,7 +43,11 @@ func (kc *KeepClient) CollectionFileReader(collection map[string]interface{}, fi
 }
 
 func (kc *KeepClient) ManifestFileReader(m manifest.Manifest, filename string) (arvados.File, error) {
-	return (&arvados.Collection{ManifestText: m.Text}).FileSystem(nil, kc).OpenFile(filename, os.O_RDONLY, 0)
+	fs, err := (&arvados.Collection{ManifestText: m.Text}).FileSystem(nil, kc)
+	if err != nil {
+		return nil, err
+	}
+	return fs.OpenFile(filename, os.O_RDONLY, 0)
 }
 
 type file struct {
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index fd36218..b4f3062 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -352,11 +352,15 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	}
 	applyContentDispositionHdr(w, r, basename, attachment)
 
-	fs := collection.FileSystem(&arvados.Client{
+	fs, err := collection.FileSystem(&arvados.Client{
 		APIHost:   arv.ApiServer,
 		AuthToken: arv.ApiToken,
 		Insecure:  arv.ApiInsecure,
 	}, kc)
+	if err != nil {
+		statusCode, statusText = http.StatusInternalServerError, err.Error()
+		return
+	}
 	if webdavMethod[r.Method] {
 		h := webdav.Handler{
 			Prefix:     "/" + strings.Join(pathParts[:stripParts], "/"),

commit ec69b9a0c19680464ca686c13e658d521fb12577
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Nov 14 00:04:24 2017 -0500

    12483: Avoid write/marshal race. Remove dead code.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 36b3bfd..eb92f16 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -607,10 +607,11 @@ func (dn *dirnode) sync() error {
 func (dn *dirnode) MarshalManifest(prefix string) (string, error) {
 	dn.Lock()
 	defer dn.Unlock()
-	if err := dn.sync(); err != nil {
-		return "", err
-	}
+	return dn.marshalManifest(prefix)
+}
 
+// caller must have read lock.
+func (dn *dirnode) marshalManifest(prefix string) (string, error) {
 	var streamLen int64
 	type m1segment struct {
 		name   string
@@ -621,9 +622,15 @@ func (dn *dirnode) MarshalManifest(prefix string) (string, error) {
 	var subdirs string
 	var blocks []string
 
+	if err := dn.sync(); err != nil {
+		return "", err
+	}
+
 	names := make([]string, 0, len(dn.inodes))
-	for name := range dn.inodes {
+	for name, node := range dn.inodes {
 		names = append(names, name)
+		node.Lock()
+		defer node.Unlock()
 	}
 	sort.Strings(names)
 
@@ -631,7 +638,7 @@ func (dn *dirnode) MarshalManifest(prefix string) (string, error) {
 		node := dn.inodes[name]
 		switch node := node.(type) {
 		case *dirnode:
-			subdir, err := node.MarshalManifest(prefix + "/" + node.Name())
+			subdir, err := node.marshalManifest(prefix + "/" + node.Name())
 			if err != nil {
 				return "", err
 			}
@@ -639,14 +646,6 @@ func (dn *dirnode) MarshalManifest(prefix string) (string, error) {
 		case *filenode:
 			for _, e := range node.extents {
 				switch e := e.(type) {
-				case *memExtent:
-					blocks = append(blocks, fmt.Sprintf("FIXME+%d", e.Len()))
-					segments = append(segments, m1segment{
-						name:   node.Name(),
-						offset: streamLen,
-						length: int64(e.Len()),
-					})
-					streamLen += int64(e.Len())
 				case storedExtent:
 					if len(blocks) > 0 && blocks[len(blocks)-1] == e.locator {
 						streamLen -= int64(e.size)
@@ -660,6 +659,9 @@ func (dn *dirnode) MarshalManifest(prefix string) (string, error) {
 					})
 					streamLen += int64(e.size)
 				default:
+					// This can't happen: we
+					// haven't unlocked since
+					// calling sync().
 					panic(fmt.Sprintf("can't marshal extent type %T", e))
 				}
 			}

commit 90fca0f2320076993c5e04afe56d5fd08e2225b6
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Nov 13 16:50:22 2017 -0500

    12453: Persist written data to keep.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index f0f8b0c..36b3bfd 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -5,7 +5,6 @@
 package arvados
 
 import (
-	"crypto/md5"
 	"errors"
 	"fmt"
 	"io"
@@ -44,6 +43,7 @@ type File interface {
 
 type keepClient interface {
 	ReadAt(locator string, p []byte, off int) (int, error)
+	PutB(p []byte) (string, int, error)
 }
 
 type fileinfo struct {
@@ -429,7 +429,6 @@ func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePt
 // FileSystem returns a CollectionFileSystem for the collection.
 func (c *Collection) FileSystem(client *Client, kc keepClient) CollectionFileSystem {
 	fs := &fileSystem{dirnode: dirnode{
-		cache:    &keepBlockCache{kc: kc},
 		client:   client,
 		kc:       kc,
 		fileinfo: fileinfo{name: ".", mode: os.ModeDir | 0755},
@@ -527,7 +526,6 @@ type dirnode struct {
 	parent *dirnode
 	client *Client
 	kc     keepClient
-	cache  blockCache
 	inodes map[string]inode
 	sync.RWMutex
 }
@@ -545,24 +543,21 @@ func (dn *dirnode) sync() error {
 		if len(sbs) == 0 {
 			return nil
 		}
-		hash := md5.New()
-		size := 0
+		block := make([]byte, 0, maxBlockSize)
 		for _, sb := range sbs {
-			data := sb.fn.extents[sb.idx].(*memExtent).buf
-			if _, err := hash.Write(data); err != nil {
-				return err
-			}
-			size += len(data)
+			block = append(block, sb.fn.extents[sb.idx].(*memExtent).buf...)
+		}
+		locator, _, err := dn.kc.PutB(block)
+		if err != nil {
+			return err
 		}
-		// FIXME: write to keep
-		locator := fmt.Sprintf("%x+%d", hash.Sum(nil), size)
 		off := 0
 		for _, sb := range sbs {
 			data := sb.fn.extents[sb.idx].(*memExtent).buf
 			sb.fn.extents[sb.idx] = storedExtent{
-				cache:   dn.cache,
+				kc:      dn.kc,
 				locator: locator,
-				size:    size,
+				size:    len(block),
 				offset:  off,
 				length:  len(data),
 			}
@@ -674,14 +669,14 @@ func (dn *dirnode) MarshalManifest(prefix string) (string, error) {
 	}
 	var filetokens []string
 	for _, s := range segments {
-		filetokens = append(filetokens, fmt.Sprintf("%d:%d:%s", s.offset, s.length, s.name))
+		filetokens = append(filetokens, fmt.Sprintf("%d:%d:%s", s.offset, s.length, manifestEscape(s.name)))
 	}
 	if len(filetokens) == 0 {
 		return subdirs, nil
 	} else if len(blocks) == 0 {
 		blocks = []string{"d41d8cd98f00b204e9800998ecf8427e+0"}
 	}
-	return prefix + " " + strings.Join(blocks, " ") + " " + strings.Join(filetokens, " ") + "\n" + subdirs, nil
+	return manifestEscape(prefix) + " " + strings.Join(blocks, " ") + " " + strings.Join(filetokens, " ") + "\n" + subdirs, nil
 }
 
 func (dn *dirnode) loadManifest(txt string) {
@@ -732,13 +727,13 @@ func (dn *dirnode) loadManifest(txt string) {
 			dn.makeParentDirs(name)
 			f, err := dn.OpenFile(name, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0700)
 			if err != nil {
-				// FIXME: broken
-				continue
+				// FIXME: don't panic
+				panic(fmt.Errorf("cannot append to %q: %s", name, err))
 			}
 			if f.inode.Stat().IsDir() {
 				f.Close()
-				// FIXME: broken manifest
-				continue
+				// FIXME: don't panic
+				panic(fmt.Errorf("cannot append to %q: is a directory", name))
 			}
 			// Map the stream offset/range coordinates to
 			// block/offset/range coordinates and add
@@ -762,7 +757,7 @@ func (dn *dirnode) loadManifest(txt string) {
 					blkLen = int(offset + length - pos - int64(blkOff))
 				}
 				f.inode.(*filenode).appendExtent(storedExtent{
-					cache:   dn.cache,
+					kc:      dn.kc,
 					locator: e.locator,
 					size:    e.size,
 					offset:  blkOff,
@@ -1015,7 +1010,7 @@ func (me *memExtent) ReadAt(p []byte, off int64) (n int, err error) {
 }
 
 type storedExtent struct {
-	cache   blockCache
+	kc      keepClient
 	locator string
 	size    int
 	offset  int
@@ -1042,27 +1037,13 @@ func (se storedExtent) ReadAt(p []byte, off int64) (n int, err error) {
 	maxlen := se.length - int(off)
 	if len(p) > maxlen {
 		p = p[:maxlen]
-		n, err = se.cache.ReadAt(se.locator, p, int(off)+se.offset)
+		n, err = se.kc.ReadAt(se.locator, p, int(off)+se.offset)
 		if err == nil {
 			err = io.EOF
 		}
 		return
 	}
-	return se.cache.ReadAt(se.locator, p, int(off)+se.offset)
-}
-
-type blockCache interface {
-	ReadAt(locator string, p []byte, off int) (n int, err error)
-}
-
-type keepBlockCache struct {
-	kc keepClient
-}
-
-var scratch = make([]byte, 2<<26)
-
-func (kbc *keepBlockCache) ReadAt(locator string, p []byte, off int) (int, error) {
-	return kbc.kc.ReadAt(locator, p, off)
+	return se.kc.ReadAt(se.locator, p, int(off)+se.offset)
 }
 
 func canonicalName(name string) string {
@@ -1077,7 +1058,7 @@ func canonicalName(name string) string {
 
 var manifestEscapeSeq = regexp.MustCompile(`\\([0-9]{3}|\\)`)
 
-func manifestUnescapeSeq(seq string) string {
+func manifestUnescapeFunc(seq string) string {
 	if seq == `\\` {
 		return `\`
 	}
@@ -1090,5 +1071,15 @@ func manifestUnescapeSeq(seq string) string {
 }
 
 func manifestUnescape(s string) string {
-	return manifestEscapeSeq.ReplaceAllStringFunc(s, manifestUnescapeSeq)
+	return manifestEscapeSeq.ReplaceAllStringFunc(s, manifestUnescapeFunc)
+}
+
+var manifestEscapedChar = regexp.MustCompile(`[^\.\w/]`)
+
+func manifestEscapeFunc(seq string) string {
+	return fmt.Sprintf("\\%03o", byte(seq[0]))
+}
+
+func manifestEscape(s string) string {
+	return manifestEscapedChar.ReplaceAllStringFunc(s, manifestEscapeFunc)
 }
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index ce3f85d..5d49ba3 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -5,6 +5,8 @@
 package arvados
 
 import (
+	"crypto/md5"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -23,16 +25,31 @@ var _ = check.Suite(&CollectionFSSuite{})
 
 type keepClientStub struct {
 	blocks map[string][]byte
+	sync.RWMutex
 }
 
+var errStub404 = errors.New("404 block not found")
+
 func (kcs *keepClientStub) ReadAt(locator string, p []byte, off int) (int, error) {
+	kcs.RLock()
+	defer kcs.RUnlock()
 	buf := kcs.blocks[locator[:32]]
 	if buf == nil {
-		return 0, os.ErrNotExist
+		return 0, errStub404
 	}
 	return copy(p, buf[off:]), nil
 }
 
+func (kcs *keepClientStub) PutB(p []byte) (string, int, error) {
+	locator := fmt.Sprintf("%x+%d+A12345 at abcde", md5.Sum(p), len(p))
+	buf := make([]byte, len(p))
+	copy(buf, p)
+	kcs.Lock()
+	defer kcs.Unlock()
+	kcs.blocks[locator[:32]] = buf
+	return locator, 1, nil
+}
+
 type CollectionFSSuite struct {
 	client *Client
 	coll   Collection
@@ -371,7 +388,7 @@ func (s *CollectionFSSuite) TestMkdir(c *check.C) {
 	// creating foo/bar as a directory should fail
 	f, err = s.fs.OpenFile("foo/bar", os.O_CREATE|os.O_EXCL, os.ModeDir)
 	c.Check(err, check.NotNil)
-	err = s.fs.Mkdir("foo/bar")
+	err = s.fs.Mkdir("foo/bar", 0755)
 	c.Check(err, check.NotNil)
 
 	m, err := s.fs.MarshalManifest(".")
@@ -422,6 +439,8 @@ func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
 	maxBlockSize = 8
 	defer func() { maxBlockSize = 2 << 26 }()
 
+	s.fs = (&Collection{}).FileSystem(s.client, s.kc)
+
 	var wg sync.WaitGroup
 	for n := 0; n < 128; n++ {
 		wg.Add(1)
@@ -467,11 +486,75 @@ func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
 	defer root.Close()
 	fi, err := root.Readdir(-1)
 	c.Check(err, check.IsNil)
-	c.Logf("Readdir(): %#v", fi)
+	c.Check(len(fi), check.Equals, 128)
+
+	_, err = s.fs.MarshalManifest(".")
+	c.Check(err, check.IsNil)
+	// TODO: check manifest content
+}
+
+func (s *CollectionFSSuite) TestPersist(c *check.C) {
+	maxBlockSize = 1024
+	defer func() { maxBlockSize = 2 << 26 }()
+
+	s.fs = (&Collection{}).FileSystem(s.client, s.kc)
+	err := s.fs.Mkdir("d:r", 0755)
+	c.Assert(err, check.IsNil)
+
+	expect := map[string][]byte{}
+
+	var wg sync.WaitGroup
+	for _, name := range []string{"random 1", "random:2", "random\\3", "d:r/random4"} {
+		buf := make([]byte, 500)
+		rand.Read(buf)
+		expect[name] = buf
+
+		f, err := s.fs.OpenFile(name, os.O_WRONLY|os.O_CREATE, 0)
+		c.Assert(err, check.IsNil)
+		// Note: we don't close the file until after the test
+		// is done. Writes to unclosed files should persist.
+		defer f.Close()
+
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for i := 0; i < len(buf); i += 5 {
+				_, err := f.Write(buf[i : i+5])
+				c.Assert(err, check.IsNil)
+			}
+		}()
+	}
+	wg.Wait()
 
 	m, err := s.fs.MarshalManifest(".")
 	c.Check(err, check.IsNil)
-	c.Logf("%s", m)
+	c.Logf("%q", m)
+
+	root, err := s.fs.Open("/")
+	c.Assert(err, check.IsNil)
+	defer root.Close()
+	fi, err := root.Readdir(-1)
+	c.Check(err, check.IsNil)
+	c.Check(len(fi), check.Equals, 4)
+
+	persisted := (&Collection{ManifestText: m}).FileSystem(s.client, s.kc)
+
+	root, err = persisted.Open("/")
+	c.Assert(err, check.IsNil)
+	defer root.Close()
+	fi, err = root.Readdir(-1)
+	c.Check(err, check.IsNil)
+	c.Check(len(fi), check.Equals, 4)
+
+	for name, content := range expect {
+		c.Logf("read %q", name)
+		f, err := persisted.Open(name)
+		c.Assert(err, check.IsNil)
+		defer f.Close()
+		buf, err := ioutil.ReadAll(f)
+		c.Check(err, check.IsNil)
+		c.Check(buf, check.DeepEquals, content)
+	}
 }
 
 // Gocheck boilerplate

commit cd45aed0312fc44046dcafe1681f5a4dc3ec1512
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Nov 13 16:50:07 2017 -0500

    12483: Remove unnecessary OpenFile() from inode interface.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index eb029fc..f0f8b0c 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -126,7 +126,6 @@ func (fs *fileSystem) Stat(name string) (os.FileInfo, error) {
 
 type inode interface {
 	os.FileInfo
-	OpenFile(string, int, os.FileMode) (*file, error)
 	Parent() inode
 	Read([]byte, filenodePtr) (int, filenodePtr, error)
 	Write([]byte, filenodePtr) (int, filenodePtr, error)
@@ -226,10 +225,6 @@ func (fn *filenode) appendExtent(e extent) {
 	fn.fileinfo.size += int64(e.Len())
 }
 
-func (fn *filenode) OpenFile(string, int, os.FileMode) (*file, error) {
-	return nil, os.ErrNotExist
-}
-
 func (fn *filenode) Parent() inode {
 	return fn.parent
 }
@@ -527,10 +522,6 @@ func (f *file) Close() error {
 	return nil
 }
 
-func (f *file) OpenFile(name string, flag int, perm os.FileMode) (*file, error) {
-	return f.inode.OpenFile(name, flag, perm)
-}
-
 type dirnode struct {
 	fileinfo
 	parent *dirnode

commit 4f21d0e5c0d2dca218bc9a204d364c15ad7450b1
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Mon Nov 13 10:36:50 2017 -0500

    12483: Add Mkdir(), Remove().
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 2f1ec2f..eb029fc 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -21,11 +21,12 @@ import (
 )
 
 var (
-	ErrReadOnlyFile     = errors.New("read-only file")
-	ErrNegativeOffset   = errors.New("cannot seek to negative offset")
-	ErrFileExists       = errors.New("file exists")
-	ErrInvalidOperation = errors.New("invalid operation")
-	ErrPermission       = os.ErrPermission
+	ErrReadOnlyFile      = errors.New("read-only file")
+	ErrNegativeOffset    = errors.New("cannot seek to negative offset")
+	ErrFileExists        = errors.New("file exists")
+	ErrInvalidOperation  = errors.New("invalid operation")
+	ErrDirectoryNotEmpty = errors.New("directory not empty")
+	ErrPermission        = os.ErrPermission
 
 	maxBlockSize = 1 << 26
 )
@@ -93,6 +94,8 @@ type CollectionFileSystem interface {
 	Stat(name string) (os.FileInfo, error)
 	Create(name string) (File, error)
 	OpenFile(name string, flag int, perm os.FileMode) (File, error)
+	Mkdir(name string, perm os.FileMode) error
+	Remove(name string) error
 	MarshalManifest(string) (string, error)
 }
 
@@ -781,37 +784,63 @@ func (dn *dirnode) loadManifest(txt string) {
 	}
 }
 
-func (dn *dirnode) makeParentDirs(name string) {
+func (dn *dirnode) makeParentDirs(name string) (err error) {
 	names := strings.Split(name, "/")
 	for _, name := range names[:len(names)-1] {
-		dn.Lock()
-		defer dn.Unlock()
-		if n, ok := dn.inodes[name]; !ok {
-			n := &dirnode{
-				parent: dn,
-				client: dn.client,
-				kc:     dn.kc,
-				fileinfo: fileinfo{
-					name: name,
-					mode: os.ModeDir | 0755,
-				},
-			}
-			if dn.inodes == nil {
-				dn.inodes = make(map[string]inode)
-			}
-			dn.inodes[name] = n
-			dn.fileinfo.size++
-			dn = n
-		} else if n, ok := n.(*dirnode); ok {
-			dn = n
-		} else {
-			// fail
-			return
+		f, err := dn.mkdir(name)
+		if err != nil {
+			return err
+		}
+		defer f.Close()
+		var ok bool
+		dn, ok = f.inode.(*dirnode)
+		if !ok {
+			return ErrFileExists
+		}
+	}
+	return nil
+}
+
+func (dn *dirnode) mkdir(name string) (*file, error) {
+	return dn.OpenFile(name, os.O_CREATE|os.O_EXCL, os.ModeDir|0755)
+}
+
+func (dn *dirnode) Mkdir(name string, perm os.FileMode) error {
+	f, err := dn.mkdir(name)
+	if err != nil {
+		f.Close()
+	}
+	return err
+}
+
+func (dn *dirnode) Remove(name string) error {
+	dirname, name := path.Split(name)
+	if name == "" || name == "." || name == ".." {
+		return ErrInvalidOperation
+	}
+	dn, ok := dn.lookupPath(dirname).(*dirnode)
+	if !ok {
+		return os.ErrNotExist
+	}
+	dn.Lock()
+	defer dn.Unlock()
+	switch node := dn.inodes[name].(type) {
+	case nil:
+		return os.ErrNotExist
+	case *dirnode:
+		node.RLock()
+		defer node.RUnlock()
+		if len(node.inodes) > 0 {
+			return ErrDirectoryNotEmpty
 		}
 	}
+	delete(dn.inodes, name)
+	return nil
 }
 
 func (dn *dirnode) Parent() inode {
+	dn.RLock()
+	defer dn.RUnlock()
 	return dn.parent
 }
 
@@ -837,42 +866,78 @@ func (dn *dirnode) Truncate(int64) error {
 	return ErrInvalidOperation
 }
 
+// lookupPath returns the inode for the file/directory with the given
+// name (which may contain "/" separators), along with its parent
+// node. If no such file/directory exists, the returned node is nil.
+func (dn *dirnode) lookupPath(path string) (node inode) {
+	node = dn
+	for _, name := range strings.Split(path, "/") {
+		dn, ok := node.(*dirnode)
+		if !ok {
+			return nil
+		}
+		if name == "." || name == "" {
+			continue
+		}
+		if name == ".." {
+			node = node.Parent()
+			continue
+		}
+		dn.RLock()
+		node = dn.inodes[name]
+		dn.RUnlock()
+	}
+	return
+}
+
 func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, error) {
-	name = strings.TrimSuffix(name, "/")
-	if name == "." || name == "" {
-		return &file{inode: dn}, nil
+	dirname, name := path.Split(name)
+	dn, ok := dn.lookupPath(dirname).(*dirnode)
+	if !ok {
+		return nil, os.ErrNotExist
 	}
-	if dirname, name := path.Split(name); dirname != "" {
-		// OpenFile("foo/bar/baz") =>
-		// OpenFile("foo/bar").OpenFile("baz") (or
-		// ErrNotExist, if foo/bar is a file)
-		f, err := dn.OpenFile(dirname, os.O_RDONLY, 0)
-		if err != nil {
-			return nil, err
-		}
-		defer f.Close()
-		if dn, ok := f.inode.(*dirnode); ok {
-			return dn.OpenFile(name, flag, perm)
-		} else {
-			return nil, os.ErrNotExist
+	writeMode := flag&(os.O_RDWR|os.O_WRONLY|os.O_CREATE) != 0
+	if !writeMode {
+		// A directory can be opened via "foo/", "foo/.", or
+		// "foo/..".
+		switch name {
+		case ".", "":
+			return &file{inode: dn}, nil
+		case "..":
+			return &file{inode: dn.Parent()}, nil
 		}
 	}
-	dn.Lock()
-	defer dn.Unlock()
-	if name == ".." {
-		return &file{inode: dn.parent}, nil
+	createMode := flag&os.O_CREATE != 0
+	if createMode {
+		dn.Lock()
+		defer dn.Unlock()
+	} else {
+		dn.RLock()
+		defer dn.RUnlock()
 	}
 	n, ok := dn.inodes[name]
 	if !ok {
-		if flag&os.O_CREATE == 0 {
+		if !createMode {
 			return nil, os.ErrNotExist
 		}
-		n = &filenode{
-			parent: dn,
-			fileinfo: fileinfo{
-				name: name,
-				mode: 0755,
-			},
+		if perm.IsDir() {
+			n = &dirnode{
+				parent: dn,
+				client: dn.client,
+				kc:     dn.kc,
+				fileinfo: fileinfo{
+					name: name,
+					mode: os.ModeDir | 0755,
+				},
+			}
+		} else {
+			n = &filenode{
+				parent: dn,
+				fileinfo: fileinfo{
+					name: name,
+					mode: 0755,
+				},
+			}
 		}
 		if dn.inodes == nil {
 			dn.inodes = make(map[string]inode)
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 70d46e2..ce3f85d 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -325,6 +325,61 @@ func (s *CollectionFSSuite) TestMarshalSmallBlocks(c *check.C) {
 	c.Check(m, check.Equals, ". c3c23db5285662ef7172373df0003206+6 acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:bar 3:3:baz 6:3:foo\n")
 }
 
+func (s *CollectionFSSuite) TestMkdir(c *check.C) {
+	err := s.fs.Mkdir("foo/bar", 0755)
+	c.Check(err, check.Equals, os.ErrNotExist)
+
+	f, err := s.fs.OpenFile("foo/bar", os.O_CREATE, 0)
+	c.Check(err, check.Equals, os.ErrNotExist)
+
+	err = s.fs.Mkdir("foo", 0755)
+	c.Check(err, check.IsNil)
+
+	f, err = s.fs.OpenFile("foo/bar", os.O_CREATE|os.O_WRONLY, 0)
+	c.Check(err, check.IsNil)
+	if err == nil {
+		defer f.Close()
+		f.Write([]byte("foo"))
+	}
+
+	// mkdir fails if a file already exists with that name
+	err = s.fs.Mkdir("foo/bar", 0755)
+	c.Check(err, check.NotNil)
+
+	err = s.fs.Remove("foo/bar")
+	c.Check(err, check.IsNil)
+
+	// mkdir succeds after the file is deleted
+	err = s.fs.Mkdir("foo/bar", 0755)
+	c.Check(err, check.IsNil)
+
+	// creating a file in a nonexistent subdir should still fail
+	f, err = s.fs.OpenFile("foo/bar/baz/foo.txt", os.O_CREATE|os.O_WRONLY, 0)
+	c.Check(err, check.Equals, os.ErrNotExist)
+
+	f, err = s.fs.OpenFile("foo/bar/foo.txt", os.O_CREATE|os.O_WRONLY, 0)
+	c.Check(err, check.IsNil)
+	if err == nil {
+		defer f.Close()
+		f.Write([]byte("foo"))
+	}
+
+	// creating foo/bar as a regular file should fail
+	f, err = s.fs.OpenFile("foo/bar", os.O_CREATE|os.O_EXCL, 0)
+	c.Check(err, check.NotNil)
+
+	// creating foo/bar as a directory should fail
+	f, err = s.fs.OpenFile("foo/bar", os.O_CREATE|os.O_EXCL, os.ModeDir)
+	c.Check(err, check.NotNil)
+	err = s.fs.Mkdir("foo/bar")
+	c.Check(err, check.NotNil)
+
+	m, err := s.fs.MarshalManifest(".")
+	c.Check(err, check.IsNil)
+	m = regexp.MustCompile(`\+A[^\+ ]+`).ReplaceAllLiteralString(m, "")
+	c.Check(m, check.Equals, "./dir1 3858f62230ac3c915f300c664312c63f+6 3:3:bar 0:3:foo\n./foo/bar acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo.txt\n")
+}
+
 func (s *CollectionFSSuite) TestConcurrentWriters(c *check.C) {
 	maxBlockSize = 8
 	defer func() { maxBlockSize = 2 << 26 }()

commit 236ede1415d7a09f97a05c5ab63e2cedd4d19d48
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sun Nov 12 03:31:16 2017 -0500

    12483: Add MarshalManifest().
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 17740db..2f1ec2f 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -5,12 +5,15 @@
 package arvados
 
 import (
+	"crypto/md5"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"os"
 	"path"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -90,6 +93,7 @@ type CollectionFileSystem interface {
 	Stat(name string) (os.FileInfo, error)
 	Create(name string) (File, error)
 	OpenFile(name string, flag int, perm os.FileMode) (File, error)
+	MarshalManifest(string) (string, error)
 }
 
 type fileSystem struct {
@@ -534,6 +538,158 @@ type dirnode struct {
 	sync.RWMutex
 }
 
+// caller must hold dn.Lock().
+func (dn *dirnode) sync() error {
+	type shortBlock struct {
+		fn  *filenode
+		idx int
+	}
+	var pending []shortBlock
+	var pendingLen int
+
+	flush := func(sbs []shortBlock) error {
+		if len(sbs) == 0 {
+			return nil
+		}
+		hash := md5.New()
+		size := 0
+		for _, sb := range sbs {
+			data := sb.fn.extents[sb.idx].(*memExtent).buf
+			if _, err := hash.Write(data); err != nil {
+				return err
+			}
+			size += len(data)
+		}
+		// FIXME: write to keep
+		locator := fmt.Sprintf("%x+%d", hash.Sum(nil), size)
+		off := 0
+		for _, sb := range sbs {
+			data := sb.fn.extents[sb.idx].(*memExtent).buf
+			sb.fn.extents[sb.idx] = storedExtent{
+				cache:   dn.cache,
+				locator: locator,
+				size:    size,
+				offset:  off,
+				length:  len(data),
+			}
+			off += len(data)
+		}
+		return nil
+	}
+
+	names := make([]string, 0, len(dn.inodes))
+	for name := range dn.inodes {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+
+	for _, name := range names {
+		fn, ok := dn.inodes[name].(*filenode)
+		if !ok {
+			continue
+		}
+		fn.Lock()
+		defer fn.Unlock()
+		for idx, ext := range fn.extents {
+			ext, ok := ext.(*memExtent)
+			if !ok {
+				continue
+			}
+			if ext.Len() > maxBlockSize/2 {
+				if err := flush([]shortBlock{{fn, idx}}); err != nil {
+					return err
+				}
+				continue
+			}
+			if pendingLen+ext.Len() > maxBlockSize {
+				if err := flush(pending); err != nil {
+					return err
+				}
+				pending = nil
+				pendingLen = 0
+			}
+			pending = append(pending, shortBlock{fn, idx})
+			pendingLen += ext.Len()
+		}
+	}
+	return flush(pending)
+}
+
+func (dn *dirnode) MarshalManifest(prefix string) (string, error) {
+	dn.Lock()
+	defer dn.Unlock()
+	if err := dn.sync(); err != nil {
+		return "", err
+	}
+
+	var streamLen int64
+	type m1segment struct {
+		name   string
+		offset int64
+		length int64
+	}
+	var segments []m1segment
+	var subdirs string
+	var blocks []string
+
+	names := make([]string, 0, len(dn.inodes))
+	for name := range dn.inodes {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+
+	for _, name := range names {
+		node := dn.inodes[name]
+		switch node := node.(type) {
+		case *dirnode:
+			subdir, err := node.MarshalManifest(prefix + "/" + node.Name())
+			if err != nil {
+				return "", err
+			}
+			subdirs = subdirs + subdir
+		case *filenode:
+			for _, e := range node.extents {
+				switch e := e.(type) {
+				case *memExtent:
+					blocks = append(blocks, fmt.Sprintf("FIXME+%d", e.Len()))
+					segments = append(segments, m1segment{
+						name:   node.Name(),
+						offset: streamLen,
+						length: int64(e.Len()),
+					})
+					streamLen += int64(e.Len())
+				case storedExtent:
+					if len(blocks) > 0 && blocks[len(blocks)-1] == e.locator {
+						streamLen -= int64(e.size)
+					} else {
+						blocks = append(blocks, e.locator)
+					}
+					segments = append(segments, m1segment{
+						name:   node.Name(),
+						offset: streamLen + int64(e.offset),
+						length: int64(e.length),
+					})
+					streamLen += int64(e.size)
+				default:
+					panic(fmt.Sprintf("can't marshal extent type %T", e))
+				}
+			}
+		default:
+			panic(fmt.Sprintf("can't marshal inode type %T", node))
+		}
+	}
+	var filetokens []string
+	for _, s := range segments {
+		filetokens = append(filetokens, fmt.Sprintf("%d:%d:%s", s.offset, s.length, s.name))
+	}
+	if len(filetokens) == 0 {
+		return subdirs, nil
+	} else if len(blocks) == 0 {
+		blocks = []string{"d41d8cd98f00b204e9800998ecf8427e+0"}
+	}
+	return prefix + " " + strings.Join(blocks, " ") + " " + strings.Join(filetokens, " ") + "\n" + subdirs, nil
+}
+
 func (dn *dirnode) loadManifest(txt string) {
 	// FIXME: faster
 	var dirname string
@@ -557,6 +713,7 @@ func (dn *dirnode) loadManifest(txt string) {
 				}
 				extents = append(extents, storedExtent{
 					locator: token,
+					size:    int(length),
 					offset:  0,
 					length:  int(length),
 				})
@@ -613,6 +770,7 @@ func (dn *dirnode) loadManifest(txt string) {
 				f.inode.(*filenode).appendExtent(storedExtent{
 					cache:   dn.cache,
 					locator: e.locator,
+					size:    e.size,
 					offset:  blkOff,
 					length:  blkLen,
 				})
@@ -803,6 +961,7 @@ func (me *memExtent) ReadAt(p []byte, off int64) (n int, err error) {
 type storedExtent struct {
 	cache   blockCache
 	locator string
+	size    int
 	offset  int
 	length  int
 }
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 4208079..70d46e2 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -11,6 +11,7 @@ import (
 	"math/rand"
 	"net/http"
 	"os"
+	"regexp"
 	"sync"
 	"testing"
 
@@ -299,6 +300,29 @@ func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
 	c.Check(err, check.IsNil)
 	c.Check(string(buf2), check.Equals, "123")
 	c.Check(len(f.(*file).inode.(*filenode).extents), check.Equals, 1)
+
+	m, err := s.fs.MarshalManifest(".")
+	c.Check(err, check.IsNil)
+	m = regexp.MustCompile(`\+A[^\+ ]+`).ReplaceAllLiteralString(m, "")
+	c.Check(m, check.Equals, "./dir1 3858f62230ac3c915f300c664312c63f+6 202cb962ac59075b964b07152d234b70+3 3:3:bar 6:3:foo\n")
+}
+
+func (s *CollectionFSSuite) TestMarshalSmallBlocks(c *check.C) {
+	maxBlockSize = 8
+	defer func() { maxBlockSize = 2 << 26 }()
+
+	s.fs = (&Collection{}).FileSystem(s.client, s.kc)
+	for _, name := range []string{"foo", "bar", "baz"} {
+		f, err := s.fs.OpenFile(name, os.O_WRONLY|os.O_CREATE, 0)
+		c.Assert(err, check.IsNil)
+		f.Write([]byte(name))
+		f.Close()
+	}
+
+	m, err := s.fs.MarshalManifest(".")
+	c.Check(err, check.IsNil)
+	m = regexp.MustCompile(`\+A[^\+ ]+`).ReplaceAllLiteralString(m, "")
+	c.Check(m, check.Equals, ". c3c23db5285662ef7172373df0003206+6 acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:bar 3:3:baz 6:3:foo\n")
 }
 
 func (s *CollectionFSSuite) TestConcurrentWriters(c *check.C) {
@@ -389,6 +413,10 @@ func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
 	fi, err := root.Readdir(-1)
 	c.Check(err, check.IsNil)
 	c.Logf("Readdir(): %#v", fi)
+
+	m, err := s.fs.MarshalManifest(".")
+	c.Check(err, check.IsNil)
+	c.Logf("%s", m)
 }
 
 // Gocheck boilerplate

commit 8bd416797eb97ed25e52611f1644fda4e7fb7eba
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sun Nov 12 03:30:18 2017 -0500

    12483: Remove unnecessary repack.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 5a3afbd..17740db 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -217,7 +217,6 @@ func (fn *filenode) appendExtent(e extent) {
 	defer fn.Unlock()
 	fn.extents = append(fn.extents, e)
 	fn.fileinfo.size += int64(e.Len())
-	fn.repacked++
 }
 
 func (fn *filenode) OpenFile(string, int, os.FileMode) (*file, error) {

commit 015e3798f9cbba4f27e1cd23b48eb396533cc0d8
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sat Nov 11 16:48:54 2017 -0500

    12483: Add tests for concurrent writers and random write sequences.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index fc2a2b9..5a3afbd 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -263,7 +263,7 @@ func (fn *filenode) Truncate(size int64) error {
 	fn.Lock()
 	defer fn.Unlock()
 	if size < fn.fileinfo.size {
-		ptr := fn.seek(filenodePtr{off: size})
+		ptr := fn.seek(filenodePtr{off: size, repacked: fn.repacked - 1})
 		if ptr.extentOff == 0 {
 			fn.extents = fn.extents[:ptr.extentIdx]
 		} else {
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index e635bb7..4208079 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -5,10 +5,13 @@
 package arvados
 
 import (
+	"fmt"
 	"io"
 	"io/ioutil"
+	"math/rand"
 	"net/http"
 	"os"
+	"sync"
 	"testing"
 
 	"git.curoverse.com/arvados.git/sdk/go/arvadostest"
@@ -298,6 +301,96 @@ func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
 	c.Check(len(f.(*file).inode.(*filenode).extents), check.Equals, 1)
 }
 
+func (s *CollectionFSSuite) TestConcurrentWriters(c *check.C) {
+	maxBlockSize = 8
+	defer func() { maxBlockSize = 2 << 26 }()
+
+	var wg sync.WaitGroup
+	for n := 0; n < 128; n++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			f, err := s.fs.OpenFile("/dir1/foo", os.O_RDWR, 0)
+			c.Assert(err, check.IsNil)
+			defer f.Close()
+			for i := 0; i < 6502; i++ {
+				switch rand.Int() & 3 {
+				case 0:
+					f.Truncate(int64(rand.Intn(64)))
+				case 1:
+					f.Seek(int64(rand.Intn(64)), os.SEEK_SET)
+				case 2:
+					_, err := f.Write([]byte("beep boop"))
+					c.Check(err, check.IsNil)
+				case 3:
+					_, err := ioutil.ReadAll(f)
+					c.Check(err, check.IsNil)
+				}
+			}
+		}()
+	}
+	wg.Wait()
+
+	f, err := s.fs.OpenFile("/dir1/foo", os.O_RDWR, 0)
+	c.Assert(err, check.IsNil)
+	defer f.Close()
+	buf, err := ioutil.ReadAll(f)
+	c.Check(err, check.IsNil)
+	c.Logf("after lots of random r/w/seek/trunc, buf is %q", buf)
+}
+
+func (s *CollectionFSSuite) TestRandomWrites(c *check.C) {
+	maxBlockSize = 8
+	defer func() { maxBlockSize = 2 << 26 }()
+
+	var wg sync.WaitGroup
+	for n := 0; n < 128; n++ {
+		wg.Add(1)
+		go func(n int) {
+			defer wg.Done()
+			expect := make([]byte, 0, 64)
+			wbytes := []byte("there's no simple explanation for anything important that any of us do")
+			f, err := s.fs.OpenFile(fmt.Sprintf("random-%d", n), os.O_RDWR|os.O_CREATE|os.O_EXCL, 0)
+			c.Assert(err, check.IsNil)
+			defer f.Close()
+			for i := 0; i < 6502; i++ {
+				trunc := rand.Intn(65)
+				woff := rand.Intn(trunc + 1)
+				wbytes = wbytes[:rand.Intn(64-woff+1)]
+				for buf, i := expect[:cap(expect)], len(expect); i < trunc; i++ {
+					buf[i] = 0
+				}
+				expect = expect[:trunc]
+				if trunc < woff+len(wbytes) {
+					expect = expect[:woff+len(wbytes)]
+				}
+				copy(expect[woff:], wbytes)
+				f.Truncate(int64(trunc))
+				pos, err := f.Seek(int64(woff), os.SEEK_SET)
+				c.Check(pos, check.Equals, int64(woff))
+				c.Check(err, check.IsNil)
+				n, err := f.Write(wbytes)
+				c.Check(n, check.Equals, len(wbytes))
+				c.Check(err, check.IsNil)
+				pos, err = f.Seek(0, os.SEEK_SET)
+				c.Check(pos, check.Equals, int64(0))
+				c.Check(err, check.IsNil)
+				buf, err := ioutil.ReadAll(f)
+				c.Check(string(buf), check.Equals, string(expect))
+				c.Check(err, check.IsNil)
+			}
+		}(n)
+	}
+	wg.Wait()
+
+	root, err := s.fs.Open("/")
+	c.Assert(err, check.IsNil)
+	defer root.Close()
+	fi, err := root.Readdir(-1)
+	c.Check(err, check.IsNil)
+	c.Logf("Readdir(): %#v", fi)
+}
+
 // Gocheck boilerplate
 func Test(t *testing.T) {
 	check.TestingT(t)

commit 9009baaf0e3aa800de8af11c741d0adc46563200
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sat Nov 11 15:32:47 2017 -0500

    12483: Add (File)Truncate().
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 1669f88..fc2a2b9 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -35,6 +35,7 @@ type File interface {
 	Size() int64
 	Readdir(int) ([]os.FileInfo, error)
 	Stat() (os.FileInfo, error)
+	Truncate(int64) error
 }
 
 type keepClient interface {
@@ -122,6 +123,7 @@ type inode interface {
 	Parent() inode
 	Read([]byte, filenodePtr) (int, filenodePtr, error)
 	Write([]byte, filenodePtr) (int, filenodePtr, error)
+	Truncate(int64) error
 	Readdir() []os.FileInfo
 	Stat() os.FileInfo
 	sync.Locker
@@ -257,6 +259,48 @@ func (fn *filenode) Read(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr
 	return
 }
 
+func (fn *filenode) Truncate(size int64) error {
+	fn.Lock()
+	defer fn.Unlock()
+	if size < fn.fileinfo.size {
+		ptr := fn.seek(filenodePtr{off: size})
+		if ptr.extentOff == 0 {
+			fn.extents = fn.extents[:ptr.extentIdx]
+		} else {
+			fn.extents = fn.extents[:ptr.extentIdx+1]
+			e := fn.extents[ptr.extentIdx]
+			if e, ok := e.(writableExtent); ok {
+				e.Truncate(ptr.extentOff)
+			} else {
+				fn.extents[ptr.extentIdx] = e.Slice(0, ptr.extentOff)
+			}
+		}
+		fn.fileinfo.size = size
+		fn.repacked++
+		return nil
+	}
+	for size > fn.fileinfo.size {
+		grow := size - fn.fileinfo.size
+		var e writableExtent
+		var ok bool
+		if len(fn.extents) == 0 {
+			e = &memExtent{}
+			fn.extents = append(fn.extents, e)
+		} else if e, ok = fn.extents[len(fn.extents)-1].(writableExtent); !ok || e.Len() >= maxBlockSize {
+			e = &memExtent{}
+			fn.extents = append(fn.extents, e)
+		} else {
+			fn.repacked++
+		}
+		if maxgrow := int64(maxBlockSize - e.Len()); maxgrow < grow {
+			grow = maxgrow
+		}
+		e.Truncate(e.Len() + int(grow))
+		fn.fileinfo.size += grow
+	}
+	return nil
+}
+
 func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr, err error) {
 	fn.Lock()
 	defer fn.Unlock()
@@ -435,6 +479,10 @@ func (f *file) Seek(off int64, whence int) (pos int64, err error) {
 	return f.ptr.off, nil
 }
 
+func (f *file) Truncate(size int64) error {
+	return f.inode.Truncate(size)
+}
+
 func (f *file) Write(p []byte) (n int, err error) {
 	if !f.writable {
 		return 0, ErrReadOnlyFile
@@ -628,6 +676,10 @@ func (dn *dirnode) Write(p []byte, ptr filenodePtr) (int, filenodePtr, error) {
 	return 0, ptr, ErrInvalidOperation
 }
 
+func (dn *dirnode) Truncate(int64) error {
+	return ErrInvalidOperation
+}
+
 func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, error) {
 	name = strings.TrimSuffix(name, "/")
 	if name == "." || name == "" {
@@ -720,6 +772,12 @@ func (me *memExtent) Truncate(n int) {
 		newbuf := make([]byte, n, newsize)
 		copy(newbuf, me.buf)
 		me.buf = newbuf
+	} else {
+		// Zero unused part when shrinking, in case we grow
+		// and start using it again later.
+		for i := n; i < len(me.buf); i++ {
+			me.buf[i] = 0
+		}
 	}
 	me.buf = me.buf[:n]
 }
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index e4eeeff..e635bb7 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -236,6 +236,66 @@ func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
 	buf2, err := ioutil.ReadAll(f2)
 	c.Check(err, check.IsNil)
 	c.Check(string(buf2), check.Equals, "f0123456789abcdefg")
+
+	// truncate to current size
+	err = f.Truncate(18)
+	f2.Seek(0, os.SEEK_SET)
+	buf2, err = ioutil.ReadAll(f2)
+	c.Check(err, check.IsNil)
+	c.Check(string(buf2), check.Equals, "f0123456789abcdefg")
+
+	// shrink to zero some data
+	f.Truncate(15)
+	f2.Seek(0, os.SEEK_SET)
+	buf2, err = ioutil.ReadAll(f2)
+	c.Check(err, check.IsNil)
+	c.Check(string(buf2), check.Equals, "f0123456789abcd")
+
+	// grow to partial block/extent
+	f.Truncate(20)
+	f2.Seek(0, os.SEEK_SET)
+	buf2, err = ioutil.ReadAll(f2)
+	c.Check(err, check.IsNil)
+	c.Check(string(buf2), check.Equals, "f0123456789abcd\x00\x00\x00\x00\x00")
+
+	f.Truncate(0)
+	f2.Write([]byte("12345678abcdefghijkl"))
+
+	// grow to block/extent boundary
+	f.Truncate(64)
+	f2.Seek(0, os.SEEK_SET)
+	buf2, err = ioutil.ReadAll(f2)
+	c.Check(err, check.IsNil)
+	c.Check(len(buf2), check.Equals, 64)
+	c.Check(len(f.(*file).inode.(*filenode).extents), check.Equals, 8)
+
+	// shrink to block/extent boundary
+	err = f.Truncate(32)
+	f2.Seek(0, os.SEEK_SET)
+	buf2, err = ioutil.ReadAll(f2)
+	c.Check(err, check.IsNil)
+	c.Check(len(buf2), check.Equals, 32)
+	c.Check(len(f.(*file).inode.(*filenode).extents), check.Equals, 4)
+
+	// shrink to partial block/extent
+	err = f.Truncate(15)
+	f2.Seek(0, os.SEEK_SET)
+	buf2, err = ioutil.ReadAll(f2)
+	c.Check(err, check.IsNil)
+	c.Check(string(buf2), check.Equals, "12345678abcdefg")
+	c.Check(len(f.(*file).inode.(*filenode).extents), check.Equals, 2)
+
+	// Truncate to size=3 while f2's ptr is at 15
+	err = f.Truncate(3)
+	c.Check(err, check.IsNil)
+	buf2, err = ioutil.ReadAll(f2)
+	c.Check(err, check.IsNil)
+	c.Check(string(buf2), check.Equals, "")
+	f2.Seek(0, os.SEEK_SET)
+	buf2, err = ioutil.ReadAll(f2)
+	c.Check(err, check.IsNil)
+	c.Check(string(buf2), check.Equals, "123")
+	c.Check(len(f.(*file).inode.(*filenode).extents), check.Equals, 1)
 }
 
 // Gocheck boilerplate

commit 6ce00fb7121813f187b555435a3f01c2aa380f93
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sat Nov 11 13:26:37 2017 -0500

    12483: Simplify extent packing, reduce type casting.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 44b3c72..1669f88 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -23,9 +23,9 @@ var (
 	ErrFileExists       = errors.New("file exists")
 	ErrInvalidOperation = errors.New("invalid operation")
 	ErrPermission       = os.ErrPermission
-)
 
-const maxBlockSize = 1 << 26
+	maxBlockSize = 1 << 26
+)
 
 type File interface {
 	io.Reader
@@ -38,7 +38,7 @@ type File interface {
 }
 
 type keepClient interface {
-	ReadAt(locator string, p []byte, off int64) (int, error)
+	ReadAt(locator string, p []byte, off int) (int, error)
 }
 
 type fileinfo struct {
@@ -177,7 +177,7 @@ func (fn *filenode) seek(startPtr filenodePtr) (ptr filenodePtr) {
 	} else if ptr.repacked == fn.repacked {
 		// extentIdx and extentOff accurately reflect ptr.off,
 		// but might have fallen off the end of an extent
-		if int64(ptr.extentOff) >= fn.extents[ptr.extentIdx].Len() {
+		if ptr.extentOff >= fn.extents[ptr.extentIdx].Len() {
 			ptr.extentIdx++
 			ptr.extentOff = 0
 		}
@@ -200,7 +200,7 @@ func (fn *filenode) seek(startPtr filenodePtr) (ptr filenodePtr) {
 		// sum(fn.extents[i].Len()) -- but that can't happen
 		// because we have ensured fn.fileinfo.size is always
 		// accurate.
-		extLen := fn.extents[ptr.extentIdx].Len()
+		extLen := int64(fn.extents[ptr.extentIdx].Len())
 		if off+extLen > ptr.off {
 			ptr.extentOff = int(ptr.off - off)
 			break
@@ -214,7 +214,7 @@ func (fn *filenode) appendExtent(e extent) {
 	fn.Lock()
 	defer fn.Unlock()
 	fn.extents = append(fn.extents, e)
-	fn.fileinfo.size += e.Len()
+	fn.fileinfo.size += int64(e.Len())
 	fn.repacked++
 }
 
@@ -246,7 +246,7 @@ func (fn *filenode) Read(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr
 	if n > 0 {
 		ptr.off += int64(n)
 		ptr.extentOff += n
-		if int64(ptr.extentOff) == fn.extents[ptr.extentIdx].Len() {
+		if ptr.extentOff == fn.extents[ptr.extentIdx].Len() {
 			ptr.extentIdx++
 			ptr.extentOff = 0
 			if ptr.extentIdx < len(fn.extents) && err == io.EOF {
@@ -280,76 +280,90 @@ func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePt
 			_, curWritable = fn.extents[cur].(writableExtent)
 		}
 		var prevAppendable bool
-		if prev >= 0 && fn.extents[prev].Len() < int64(maxBlockSize) {
+		if prev >= 0 && fn.extents[prev].Len() < maxBlockSize {
 			_, prevAppendable = fn.extents[prev].(writableExtent)
 		}
-		if ptr.extentOff > 0 {
-			if !curWritable {
-				// Split a non-writable block.
-				if max := int(fn.extents[cur].Len()) - ptr.extentOff; max <= len(cando) {
-					cando = cando[:max]
-					fn.extents = append(fn.extents, nil)
-					copy(fn.extents[cur+1:], fn.extents[cur:])
-				} else {
-					fn.extents = append(fn.extents, nil, nil)
-					copy(fn.extents[cur+2:], fn.extents[cur:])
-					fn.extents[cur+2] = fn.extents[cur+2].Slice(ptr.extentOff+len(cando), -1)
-				}
-				cur++
-				prev++
-				e := &memExtent{}
-				e.Truncate(len(cando))
-				fn.extents[cur] = e
-				fn.extents[prev] = fn.extents[prev].Slice(0, ptr.extentOff)
-				ptr.extentIdx++
-				ptr.extentOff = 0
-				fn.repacked++
-				ptr.repacked++
+		if ptr.extentOff > 0 && !curWritable {
+			// Split a non-writable block.
+			if max := fn.extents[cur].Len() - ptr.extentOff; max <= len(cando) {
+				// Truncate cur, and insert a new
+				// extent after it.
+				cando = cando[:max]
+				fn.extents = append(fn.extents, nil)
+				copy(fn.extents[cur+1:], fn.extents[cur:])
+			} else {
+				// Split cur into two copies, truncate
+				// the one on the left, shift the one
+				// on the right, and insert a new
+				// extent between them.
+				fn.extents = append(fn.extents, nil, nil)
+				copy(fn.extents[cur+2:], fn.extents[cur:])
+				fn.extents[cur+2] = fn.extents[cur+2].Slice(ptr.extentOff+len(cando), -1)
 			}
-		} else if len(fn.extents) == 0 {
-			// File has no extents yet.
+			cur++
+			prev++
 			e := &memExtent{}
 			e.Truncate(len(cando))
-			fn.fileinfo.size += e.Len()
-			fn.extents = append(fn.extents, e)
+			fn.extents[cur] = e
+			fn.extents[prev] = fn.extents[prev].Slice(0, ptr.extentOff)
+			ptr.extentIdx++
+			ptr.extentOff = 0
+			fn.repacked++
+			ptr.repacked++
 		} else if curWritable {
 			if fit := int(fn.extents[cur].Len()) - ptr.extentOff; fit < len(cando) {
 				cando = cando[:fit]
 			}
 		} else {
 			if prevAppendable {
-				// Grow prev.
-				if cangrow := int(maxBlockSize - fn.extents[prev].Len()); cangrow < len(cando) {
+				// Shrink cando if needed to fit in prev extent.
+				if cangrow := maxBlockSize - fn.extents[prev].Len(); cangrow < len(cando) {
 					cando = cando[:cangrow]
 				}
-				ptr.extentIdx--
-				ptr.extentOff = int(fn.extents[prev].Len())
-				fn.extents[prev].(*memExtent).Truncate(ptr.extentOff + len(cando))
-			} else {
-				// Insert an extent between prev and cur. It will be the new prev.
-				fn.extents = append(fn.extents, nil)
-				copy(fn.extents[cur+1:], fn.extents[cur:])
-				e := &memExtent{}
-				e.Truncate(len(cando))
-				fn.extents[cur] = e
-				cur++
-				prev++
 			}
 
 			if cur == len(fn.extents) {
-				// There is no cur.
-			} else if el := int(fn.extents[cur].Len()); el <= len(cando) {
-				// Drop cur.
+				// ptr is at EOF, filesize is changing.
+				fn.fileinfo.size += int64(len(cando))
+			} else if el := fn.extents[cur].Len(); el <= len(cando) {
+				// cando is long enough that we won't
+				// need cur any more. shrink cando to
+				// be exactly as long as cur
+				// (otherwise we'd accidentally shift
+				// the effective position of all
+				// extents after cur).
 				cando = cando[:el]
 				copy(fn.extents[cur:], fn.extents[cur+1:])
 				fn.extents = fn.extents[:len(fn.extents)-1]
 			} else {
-				// Shrink cur.
+				// shrink cur by the same #bytes we're growing prev
 				fn.extents[cur] = fn.extents[cur].Slice(len(cando), -1)
 			}
 
-			ptr.repacked++
-			fn.repacked++
+			if prevAppendable {
+				// Grow prev.
+				ptr.extentIdx--
+				ptr.extentOff = fn.extents[prev].Len()
+				fn.extents[prev].(writableExtent).Truncate(ptr.extentOff + len(cando))
+				ptr.repacked++
+				fn.repacked++
+			} else {
+				// Insert an extent between prev and cur, and advance prev/cur.
+				fn.extents = append(fn.extents, nil)
+				if cur < len(fn.extents) {
+					copy(fn.extents[cur+1:], fn.extents[cur:])
+					ptr.repacked++
+					fn.repacked++
+				} else {
+					// appending a new extent does
+					// not invalidate any ptrs
+				}
+				e := &memExtent{}
+				e.Truncate(len(cando))
+				fn.extents[cur] = e
+				cur++
+				prev++
+			}
 		}
 
 		// Finally we can copy bytes from cando to the current extent.
@@ -359,7 +373,7 @@ func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePt
 
 		ptr.off += int64(len(cando))
 		ptr.extentOff += len(cando)
-		if fn.extents[ptr.extentIdx].Len() == int64(ptr.extentOff) {
+		if fn.extents[ptr.extentIdx].Len() == ptr.extentOff {
 			ptr.extentOff = 0
 			ptr.extentIdx++
 		}
@@ -528,10 +542,14 @@ func (dn *dirnode) loadManifest(txt string) {
 				// FIXME: broken manifest
 				continue
 			}
+			// Map the stream offset/range coordinates to
+			// block/offset/range coordinates and add
+			// corresponding storedExtents to the filenode
 			var pos int64
 			for _, e := range extents {
-				if pos+e.Len() < offset {
-					pos += e.Len()
+				next := pos + int64(e.Len())
+				if next < offset {
+					pos = next
 					continue
 				}
 				if pos > offset+length {
@@ -541,7 +559,7 @@ func (dn *dirnode) loadManifest(txt string) {
 				if pos < offset {
 					blkOff = int(offset - pos)
 				}
-				blkLen := int(e.Len()) - blkOff
+				blkLen := e.Len() - blkOff
 				if pos+int64(blkOff+blkLen) > offset+length {
 					blkLen = int(offset + length - pos - int64(blkOff))
 				}
@@ -551,7 +569,7 @@ func (dn *dirnode) loadManifest(txt string) {
 					offset:  blkOff,
 					length:  blkLen,
 				})
-				pos += e.Len()
+				pos = next
 			}
 			f.Close()
 		}
@@ -664,8 +682,10 @@ func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, err
 
 type extent interface {
 	io.ReaderAt
-	Len() int64
-	Slice(int, int) extent
+	Len() int
+	// Return a new extent with a subsection of the data from this
+	// one. length<0 means length=Len()-off.
+	Slice(off int, length int) extent
 }
 
 type writableExtent interface {
@@ -678,15 +698,17 @@ type memExtent struct {
 	buf []byte
 }
 
-func (me *memExtent) Len() int64 {
-	return int64(len(me.buf))
+func (me *memExtent) Len() int {
+	return len(me.buf)
 }
 
-func (me *memExtent) Slice(n, size int) extent {
-	if size < 0 {
-		size = len(me.buf) - n
+func (me *memExtent) Slice(off, length int) extent {
+	if length < 0 {
+		length = len(me.buf) - off
 	}
-	return &memExtent{buf: me.buf[n : n+size]}
+	buf := make([]byte, length)
+	copy(buf, me.buf[off:])
+	return &memExtent{buf: buf}
 }
 
 func (me *memExtent) Truncate(n int) {
@@ -710,7 +732,7 @@ func (me *memExtent) WriteAt(p []byte, off int) {
 }
 
 func (me *memExtent) ReadAt(p []byte, off int64) (n int, err error) {
-	if off > me.Len() {
+	if off > int64(me.Len()) {
 		err = io.EOF
 		return
 	}
@@ -728,8 +750,8 @@ type storedExtent struct {
 	length  int
 }
 
-func (se storedExtent) Len() int64 {
-	return int64(se.length)
+func (se storedExtent) Len() int {
+	return se.length
 }
 
 func (se storedExtent) Slice(n, size int) extent {
@@ -742,20 +764,23 @@ func (se storedExtent) Slice(n, size int) extent {
 }
 
 func (se storedExtent) ReadAt(p []byte, off int64) (n int, err error) {
-	maxlen := int(int64(se.length) - off)
+	if off > int64(se.length) {
+		return 0, io.EOF
+	}
+	maxlen := se.length - int(off)
 	if len(p) > maxlen {
 		p = p[:maxlen]
-		n, err = se.cache.ReadAt(se.locator, p, off+int64(se.offset))
+		n, err = se.cache.ReadAt(se.locator, p, int(off)+se.offset)
 		if err == nil {
 			err = io.EOF
 		}
 		return
 	}
-	return se.cache.ReadAt(se.locator, p, off+int64(se.offset))
+	return se.cache.ReadAt(se.locator, p, int(off)+se.offset)
 }
 
 type blockCache interface {
-	ReadAt(locator string, p []byte, off int64) (n int, err error)
+	ReadAt(locator string, p []byte, off int) (n int, err error)
 }
 
 type keepBlockCache struct {
@@ -764,7 +789,7 @@ type keepBlockCache struct {
 
 var scratch = make([]byte, 2<<26)
 
-func (kbc *keepBlockCache) ReadAt(locator string, p []byte, off int64) (int, error) {
+func (kbc *keepBlockCache) ReadAt(locator string, p []byte, off int) (int, error) {
 	return kbc.kc.ReadAt(locator, p, off)
 }
 
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index ae57078..e4eeeff 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -21,12 +21,12 @@ type keepClientStub struct {
 	blocks map[string][]byte
 }
 
-func (kcs *keepClientStub) ReadAt(locator string, p []byte, off int64) (int, error) {
+func (kcs *keepClientStub) ReadAt(locator string, p []byte, off int) (int, error) {
 	buf := kcs.blocks[locator[:32]]
 	if buf == nil {
 		return 0, os.ErrNotExist
 	}
-	return copy(p, buf[int(off):]), nil
+	return copy(p, buf[off:]), nil
 }
 
 type CollectionFSSuite struct {
@@ -177,13 +177,21 @@ func (s *CollectionFSSuite) TestCreateFile(c *check.C) {
 }
 
 func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
+	maxBlockSize = 8
+	defer func() { maxBlockSize = 2 << 26 }()
+
 	f, err := s.fs.OpenFile("/dir1/foo", os.O_RDWR, 0)
 	c.Assert(err, check.IsNil)
+	defer f.Close()
 	st, err := f.Stat()
 	c.Assert(err, check.IsNil)
 	c.Check(st.Size(), check.Equals, int64(3))
 
-	buf := make([]byte, 4)
+	f2, err := s.fs.OpenFile("/dir1/foo", os.O_RDWR, 0)
+	c.Assert(err, check.IsNil)
+	defer f2.Close()
+
+	buf := make([]byte, 64)
 	n, err := f.Read(buf)
 	c.Check(n, check.Equals, 3)
 	c.Check(err, check.Equals, io.EOF)
@@ -193,6 +201,7 @@ func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
 	c.Check(pos, check.Equals, int64(1))
 	c.Check(err, check.IsNil)
 
+	// Split a storedExtent in two, and insert a memExtent
 	n, err = f.Write([]byte("*"))
 	c.Check(n, check.Equals, 1)
 	c.Check(err, check.IsNil)
@@ -205,11 +214,28 @@ func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
 	c.Check(pos, check.Equals, int64(0))
 	c.Check(err, check.IsNil)
 
-	buf = make([]byte, 4)
-	buf, err = ioutil.ReadAll(f)
-	c.Check(len(buf), check.Equals, 3)
+	rbuf, err := ioutil.ReadAll(f)
+	c.Check(len(rbuf), check.Equals, 3)
+	c.Check(err, check.IsNil)
+	c.Check(string(rbuf), check.Equals, "f*o")
+
+	// Write multiple blocks in one call
+	f.Seek(1, os.SEEK_SET)
+	n, err = f.Write([]byte("0123456789abcdefg"))
+	c.Check(n, check.Equals, 17)
+	c.Check(err, check.IsNil)
+	pos, err = f.Seek(0, os.SEEK_CUR)
+	c.Check(pos, check.Equals, int64(18))
+	pos, err = f.Seek(-18, os.SEEK_CUR)
+	c.Check(err, check.IsNil)
+	n, err = io.ReadFull(f, buf)
+	c.Check(n, check.Equals, 18)
+	c.Check(err, check.Equals, io.ErrUnexpectedEOF)
+	c.Check(string(buf[:n]), check.Equals, "f0123456789abcdefg")
+
+	buf2, err := ioutil.ReadAll(f2)
 	c.Check(err, check.IsNil)
-	c.Check(string(buf), check.Equals, "f*o")
+	c.Check(string(buf2), check.Equals, "f0123456789abcdefg")
 }
 
 // Gocheck boilerplate
diff --git a/sdk/go/keepclient/block_cache.go b/sdk/go/keepclient/block_cache.go
index c1138fa..539975e 100644
--- a/sdk/go/keepclient/block_cache.go
+++ b/sdk/go/keepclient/block_cache.go
@@ -51,15 +51,15 @@ func (c *BlockCache) Sweep() {
 
 // ReadAt returns data from the cache, first retrieving it from Keep if
 // necessary.
-func (c *BlockCache) ReadAt(kc *KeepClient, locator string, p []byte, off int64) (int, error) {
+func (c *BlockCache) ReadAt(kc *KeepClient, locator string, p []byte, off int) (int, error) {
 	buf, err := c.Get(kc, locator)
 	if err != nil {
 		return 0, err
 	}
-	if off > int64(len(buf)) {
+	if off > len(buf) {
 		return 0, io.ErrUnexpectedEOF
 	}
-	return copy(p, buf[int(off):]), nil
+	return copy(p, buf[off:]), nil
 }
 
 // Get returns data from the cache, first retrieving it from Keep if
diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index c3d63ed..4bc0fc5 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -294,7 +294,7 @@ func (kc *KeepClient) Get(locator string) (io.ReadCloser, int64, string, error)
 
 // ReadAt() retrieves a portion of block from the cache if it's
 // present, otherwise from the network.
-func (kc *KeepClient) ReadAt(locator string, p []byte, off int64) (int, error) {
+func (kc *KeepClient) ReadAt(locator string, p []byte, off int) (int, error) {
 	return kc.cache().ReadAt(kc, locator, p, off)
 }
 

commit cad7d333436703d48c2811de8a26caef9fc130ad
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Sat Nov 11 03:30:24 2017 -0500

    12483: Implement some writable cases.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 1dee6f1..44b3c72 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -25,6 +25,8 @@ var (
 	ErrPermission       = os.ErrPermission
 )
 
+const maxBlockSize = 1 << 26
+
 type File interface {
 	io.Reader
 	io.Writer
@@ -263,7 +265,105 @@ func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePt
 		err = ErrNegativeOffset
 		return
 	}
-	err = ErrReadOnlyFile
+	for len(p) > 0 && err == nil {
+		cando := p
+		if len(cando) > maxBlockSize {
+			cando = cando[:maxBlockSize]
+		}
+		// Rearrange/grow fn.extents (and shrink cando if
+		// needed) such that cando can be copied to
+		// fn.extents[ptr.extentIdx] at offset ptr.extentOff.
+		cur := ptr.extentIdx
+		prev := ptr.extentIdx - 1
+		var curWritable bool
+		if cur < len(fn.extents) {
+			_, curWritable = fn.extents[cur].(writableExtent)
+		}
+		var prevAppendable bool
+		if prev >= 0 && fn.extents[prev].Len() < int64(maxBlockSize) {
+			_, prevAppendable = fn.extents[prev].(writableExtent)
+		}
+		if ptr.extentOff > 0 {
+			if !curWritable {
+				// Split a non-writable block.
+				if max := int(fn.extents[cur].Len()) - ptr.extentOff; max <= len(cando) {
+					cando = cando[:max]
+					fn.extents = append(fn.extents, nil)
+					copy(fn.extents[cur+1:], fn.extents[cur:])
+				} else {
+					fn.extents = append(fn.extents, nil, nil)
+					copy(fn.extents[cur+2:], fn.extents[cur:])
+					fn.extents[cur+2] = fn.extents[cur+2].Slice(ptr.extentOff+len(cando), -1)
+				}
+				cur++
+				prev++
+				e := &memExtent{}
+				e.Truncate(len(cando))
+				fn.extents[cur] = e
+				fn.extents[prev] = fn.extents[prev].Slice(0, ptr.extentOff)
+				ptr.extentIdx++
+				ptr.extentOff = 0
+				fn.repacked++
+				ptr.repacked++
+			}
+		} else if len(fn.extents) == 0 {
+			// File has no extents yet.
+			e := &memExtent{}
+			e.Truncate(len(cando))
+			fn.fileinfo.size += e.Len()
+			fn.extents = append(fn.extents, e)
+		} else if curWritable {
+			if fit := int(fn.extents[cur].Len()) - ptr.extentOff; fit < len(cando) {
+				cando = cando[:fit]
+			}
+		} else {
+			if prevAppendable {
+				// Grow prev.
+				if cangrow := int(maxBlockSize - fn.extents[prev].Len()); cangrow < len(cando) {
+					cando = cando[:cangrow]
+				}
+				ptr.extentIdx--
+				ptr.extentOff = int(fn.extents[prev].Len())
+				fn.extents[prev].(*memExtent).Truncate(ptr.extentOff + len(cando))
+			} else {
+				// Insert an extent between prev and cur. It will be the new prev.
+				fn.extents = append(fn.extents, nil)
+				copy(fn.extents[cur+1:], fn.extents[cur:])
+				e := &memExtent{}
+				e.Truncate(len(cando))
+				fn.extents[cur] = e
+				cur++
+				prev++
+			}
+
+			if cur == len(fn.extents) {
+				// There is no cur.
+			} else if el := int(fn.extents[cur].Len()); el <= len(cando) {
+				// Drop cur.
+				cando = cando[:el]
+				copy(fn.extents[cur:], fn.extents[cur+1:])
+				fn.extents = fn.extents[:len(fn.extents)-1]
+			} else {
+				// Shrink cur.
+				fn.extents[cur] = fn.extents[cur].Slice(len(cando), -1)
+			}
+
+			ptr.repacked++
+			fn.repacked++
+		}
+
+		// Finally we can copy bytes from cando to the current extent.
+		fn.extents[ptr.extentIdx].(writableExtent).WriteAt(cando, ptr.extentOff)
+		n += len(cando)
+		p = p[len(cando):]
+
+		ptr.off += int64(len(cando))
+		ptr.extentOff += len(cando)
+		if fn.extents[ptr.extentIdx].Len() == int64(ptr.extentOff) {
+			ptr.extentOff = 0
+			ptr.extentIdx++
+		}
+	}
 	return
 }
 
@@ -322,6 +422,9 @@ func (f *file) Seek(off int64, whence int) (pos int64, err error) {
 }
 
 func (f *file) Write(p []byte) (n int, err error) {
+	if !f.writable {
+		return 0, ErrReadOnlyFile
+	}
 	n, f.ptr, err = f.inode.Write(p, f.ptr)
 	return
 }
@@ -562,6 +665,7 @@ func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, err
 type extent interface {
 	io.ReaderAt
 	Len() int64
+	Slice(int, int) extent
 }
 
 type writableExtent interface {
@@ -578,6 +682,13 @@ func (me *memExtent) Len() int64 {
 	return int64(len(me.buf))
 }
 
+func (me *memExtent) Slice(n, size int) extent {
+	if size < 0 {
+		size = len(me.buf) - n
+	}
+	return &memExtent{buf: me.buf[n : n+size]}
+}
+
 func (me *memExtent) Truncate(n int) {
 	if n > cap(me.buf) {
 		newsize := 1024
@@ -621,8 +732,17 @@ func (se storedExtent) Len() int64 {
 	return int64(se.length)
 }
 
+func (se storedExtent) Slice(n, size int) extent {
+	se.offset += n
+	se.length -= n
+	if size >= 0 && se.length > size {
+		se.length = size
+	}
+	return se
+}
+
 func (se storedExtent) ReadAt(p []byte, off int64) (n int, err error) {
-	maxlen := int(int64(se.length) - int64(se.offset) - off)
+	maxlen := int(int64(se.length) - off)
 	if len(p) > maxlen {
 		p = p[:maxlen]
 		n, err = se.cache.ReadAt(se.locator, p, off+int64(se.offset))
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 605c41c..ae57078 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -6,6 +6,7 @@ package arvados
 
 import (
 	"io"
+	"io/ioutil"
 	"net/http"
 	"os"
 	"testing"
@@ -205,10 +206,10 @@ func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
 	c.Check(err, check.IsNil)
 
 	buf = make([]byte, 4)
-	n, err = f.Read(buf)
-	c.Check(n, check.Equals, 3)
-	c.Check(err, check.Equals, io.EOF)
-	c.Check(string(buf[:3]), check.Equals, "f*o")
+	buf, err = ioutil.ReadAll(f)
+	c.Check(len(buf), check.Equals, 3)
+	c.Check(err, check.IsNil)
+	c.Check(string(buf), check.Equals, "f*o")
 }
 
 // Gocheck boilerplate

commit da3f22835804cc1bafe7daf373867ef46cbb20e8
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Nov 10 16:34:28 2017 -0500

    12483: Add basic file writing tests.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 7bd3570..605c41c 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -135,14 +135,80 @@ func (s *CollectionFSSuite) TestNotExist(c *check.C) {
 	}
 }
 
-func (s *CollectionFSSuite) TestOpenFile(c *check.C) {
-	c.Skip("cannot test files with nil keepclient")
+func (s *CollectionFSSuite) TestReadOnlyFile(c *check.C) {
+	f, err := s.fs.OpenFile("/dir1/foo", os.O_RDONLY, 0)
+	c.Assert(err, check.IsNil)
+	st, err := f.Stat()
+	c.Assert(err, check.IsNil)
+	c.Check(st.Size(), check.Equals, int64(3))
+	n, err := f.Write([]byte("bar"))
+	c.Check(n, check.Equals, 0)
+	c.Check(err, check.Equals, ErrReadOnlyFile)
+}
+
+func (s *CollectionFSSuite) TestCreateFile(c *check.C) {
+	f, err := s.fs.OpenFile("/newfile", os.O_RDWR|os.O_CREATE, 0)
+	c.Assert(err, check.IsNil)
+	st, err := f.Stat()
+	c.Assert(err, check.IsNil)
+	c.Check(st.Size(), check.Equals, int64(0))
+
+	n, err := f.Write([]byte("bar"))
+	c.Check(n, check.Equals, 3)
+	c.Check(err, check.IsNil)
+
+	c.Check(f.Close(), check.IsNil)
+
+	f, err = s.fs.OpenFile("/newfile", os.O_RDWR|os.O_CREATE|os.O_EXCL, 0)
+	c.Check(f, check.IsNil)
+	c.Assert(err, check.NotNil)
+
+	f, err = s.fs.OpenFile("/newfile", os.O_RDWR, 0)
+	c.Assert(err, check.IsNil)
+	st, err = f.Stat()
+	c.Assert(err, check.IsNil)
+	c.Check(st.Size(), check.Equals, int64(3))
+
+	c.Check(f.Close(), check.IsNil)
+
+	// TODO: serialize to Collection, confirm manifest contents,
+	// make new FileSystem, confirm file contents.
+}
 
-	f, err := s.fs.Open("/foo.txt")
+func (s *CollectionFSSuite) TestReadWriteFile(c *check.C) {
+	f, err := s.fs.OpenFile("/dir1/foo", os.O_RDWR, 0)
 	c.Assert(err, check.IsNil)
 	st, err := f.Stat()
 	c.Assert(err, check.IsNil)
 	c.Check(st.Size(), check.Equals, int64(3))
+
+	buf := make([]byte, 4)
+	n, err := f.Read(buf)
+	c.Check(n, check.Equals, 3)
+	c.Check(err, check.Equals, io.EOF)
+	c.Check(string(buf[:3]), check.DeepEquals, "foo")
+
+	pos, err := f.Seek(-2, os.SEEK_CUR)
+	c.Check(pos, check.Equals, int64(1))
+	c.Check(err, check.IsNil)
+
+	n, err = f.Write([]byte("*"))
+	c.Check(n, check.Equals, 1)
+	c.Check(err, check.IsNil)
+
+	pos, err = f.Seek(0, os.SEEK_CUR)
+	c.Check(pos, check.Equals, int64(2))
+	c.Check(err, check.IsNil)
+
+	pos, err = f.Seek(0, os.SEEK_SET)
+	c.Check(pos, check.Equals, int64(0))
+	c.Check(err, check.IsNil)
+
+	buf = make([]byte, 4)
+	n, err = f.Read(buf)
+	c.Check(n, check.Equals, 3)
+	c.Check(err, check.Equals, io.EOF)
+	c.Check(string(buf[:3]), check.Equals, "f*o")
 }
 
 // Gocheck boilerplate

commit 3e3abb01d17b0968e22e6738da12c86ad0a2a06c
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Thu Nov 9 13:24:34 2017 -0500

    12483: Rewrite collection filesystem.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/sdk/go/arvados/collection_fs.go b/sdk/go/arvados/collection_fs.go
index 1acf274..1dee6f1 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -5,268 +5,647 @@
 package arvados
 
 import (
+	"errors"
 	"io"
 	"net/http"
 	"os"
 	"path"
+	"regexp"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
+)
 
-	"git.curoverse.com/arvados.git/sdk/go/manifest"
+var (
+	ErrReadOnlyFile     = errors.New("read-only file")
+	ErrNegativeOffset   = errors.New("cannot seek to negative offset")
+	ErrFileExists       = errors.New("file exists")
+	ErrInvalidOperation = errors.New("invalid operation")
+	ErrPermission       = os.ErrPermission
 )
 
 type File interface {
 	io.Reader
+	io.Writer
 	io.Closer
 	io.Seeker
 	Size() int64
+	Readdir(int) ([]os.FileInfo, error)
+	Stat() (os.FileInfo, error)
 }
 
 type keepClient interface {
-	ManifestFileReader(manifest.Manifest, string) (File, error)
+	ReadAt(locator string, p []byte, off int64) (int, error)
 }
 
-type collectionFile struct {
-	File
-	collection *Collection
-	name       string
-	size       int64
+type fileinfo struct {
+	name    string
+	mode    os.FileMode
+	size    int64
+	modTime time.Time
 }
 
-func (cf *collectionFile) Size() int64 {
-	return cf.size
+// Name implements os.FileInfo.
+func (fi fileinfo) Name() string {
+	return fi.name
 }
 
-func (cf *collectionFile) Readdir(count int) ([]os.FileInfo, error) {
-	return nil, io.EOF
+// ModTime implements os.FileInfo.
+func (fi fileinfo) ModTime() time.Time {
+	return fi.modTime
 }
 
-func (cf *collectionFile) Stat() (os.FileInfo, error) {
-	return collectionDirent{
-		collection: cf.collection,
-		name:       cf.name,
-		size:       cf.size,
-		isDir:      false,
-	}, nil
+// Mode implements os.FileInfo.
+func (fi fileinfo) Mode() os.FileMode {
+	return fi.mode
 }
 
-type collectionDir struct {
-	collection *Collection
-	stream     string
-	dirents    []os.FileInfo
+// IsDir implements os.FileInfo.
+func (fi fileinfo) IsDir() bool {
+	return fi.mode&os.ModeDir != 0
 }
 
-// Readdir implements os.File.
-func (cd *collectionDir) Readdir(count int) ([]os.FileInfo, error) {
-	ret := cd.dirents
-	if count <= 0 {
-		cd.dirents = nil
-		return ret, nil
-	} else if len(ret) == 0 {
-		return nil, io.EOF
-	}
-	var err error
-	if count >= len(ret) {
-		count = len(ret)
-		err = io.EOF
-	}
-	cd.dirents = cd.dirents[count:]
-	return ret[:count], err
+// Size implements os.FileInfo.
+func (fi fileinfo) Size() int64 {
+	return fi.size
 }
 
-// Stat implements os.File.
-func (cd *collectionDir) Stat() (os.FileInfo, error) {
-	return collectionDirent{
-		collection: cd.collection,
-		name:       path.Base(cd.stream),
-		isDir:      true,
-		size:       int64(len(cd.dirents)),
-	}, nil
+// Sys implements os.FileInfo.
+func (fi fileinfo) Sys() interface{} {
+	return nil
 }
 
-// Close implements os.File.
-func (cd *collectionDir) Close() error {
-	return nil
+func (fi fileinfo) Stat() os.FileInfo {
+	return fi
 }
 
-// Read implements os.File.
-func (cd *collectionDir) Read([]byte) (int, error) {
-	return 0, nil
+// A CollectionFileSystem is an http.Filesystem plus Stat() and
+// support for opening writable files.
+type CollectionFileSystem interface {
+	http.FileSystem
+	Stat(name string) (os.FileInfo, error)
+	Create(name string) (File, error)
+	OpenFile(name string, flag int, perm os.FileMode) (File, error)
 }
 
-// Seek implements os.File.
-func (cd *collectionDir) Seek(int64, int) (int64, error) {
-	return 0, nil
+type fileSystem struct {
+	dirnode
 }
 
-// collectionDirent implements os.FileInfo.
-type collectionDirent struct {
-	collection *Collection
-	name       string
-	isDir      bool
-	mode       os.FileMode
-	size       int64
+func (fs *fileSystem) OpenFile(name string, flag int, perm os.FileMode) (File, error) {
+	return fs.dirnode.OpenFile(path.Clean(name), flag, perm)
 }
 
-// Name implements os.FileInfo.
-func (e collectionDirent) Name() string {
-	return e.name
+func (fs *fileSystem) Open(name string) (http.File, error) {
+	return fs.dirnode.OpenFile(path.Clean(name), os.O_RDONLY, 0)
 }
 
-// ModTime implements os.FileInfo.
-func (e collectionDirent) ModTime() time.Time {
-	if e.collection.ModifiedAt == nil {
-		return time.Now()
+func (fs *fileSystem) Create(name string) (File, error) {
+	return fs.dirnode.OpenFile(path.Clean(name), os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0)
+}
+
+func (fs *fileSystem) Stat(name string) (os.FileInfo, error) {
+	f, err := fs.OpenFile(name, os.O_RDONLY, 0)
+	if err != nil {
+		return nil, err
 	}
-	return *e.collection.ModifiedAt
+	defer f.Close()
+	return f.Stat()
 }
 
-// Mode implements os.FileInfo.
-func (e collectionDirent) Mode() os.FileMode {
-	if e.isDir {
-		return 0555
-	} else {
-		return 0444
+type inode interface {
+	os.FileInfo
+	OpenFile(string, int, os.FileMode) (*file, error)
+	Parent() inode
+	Read([]byte, filenodePtr) (int, filenodePtr, error)
+	Write([]byte, filenodePtr) (int, filenodePtr, error)
+	Readdir() []os.FileInfo
+	Stat() os.FileInfo
+	sync.Locker
+	RLock()
+	RUnlock()
+}
+
+// filenode implements inode.
+type filenode struct {
+	fileinfo
+	parent   *dirnode
+	extents  []extent
+	repacked int64 // number of times anything in []extents has changed len
+	sync.RWMutex
+}
+
+// filenodePtr is an offset into a file that is (usually) efficient to
+// seek to. Specifically, if filenode.repacked==filenodePtr.repacked
+// then filenode.extents[filenodePtr.extentIdx][filenodePtr.extentOff]
+// corresponds to file offset filenodePtr.off. Otherwise, it is
+// necessary to reexamine len(filenode.extents[0]) etc. to find the
+// correct extent and offset.
+type filenodePtr struct {
+	off       int64
+	extentIdx int
+	extentOff int
+	repacked  int64
+}
+
+// seek returns a ptr that is consistent with both startPtr.off and
+// the current state of fn. The caller must already hold fn.RLock() or
+// fn.Lock().
+//
+// If startPtr points beyond the end of the file, ptr will point to
+// exactly the end of the file.
+//
+// After seeking:
+//
+//     ptr.extentIdx == len(filenode.extents) // i.e., at EOF
+//     ||
+//     filenode.extents[ptr.extentIdx].Len() >= ptr.extentOff
+func (fn *filenode) seek(startPtr filenodePtr) (ptr filenodePtr) {
+	ptr = startPtr
+	if ptr.off < 0 {
+		// meaningless anyway
+		return
+	} else if ptr.off >= fn.fileinfo.size {
+		ptr.off = fn.fileinfo.size
+		ptr.extentIdx = len(fn.extents)
+		ptr.extentOff = 0
+		ptr.repacked = fn.repacked
+		return
+	} else if ptr.repacked == fn.repacked {
+		// extentIdx and extentOff accurately reflect ptr.off,
+		// but might have fallen off the end of an extent
+		if int64(ptr.extentOff) >= fn.extents[ptr.extentIdx].Len() {
+			ptr.extentIdx++
+			ptr.extentOff = 0
+		}
+		return
+	}
+	defer func() {
+		ptr.repacked = fn.repacked
+	}()
+	if ptr.off >= fn.fileinfo.size {
+		ptr.extentIdx, ptr.extentOff = len(fn.extents), 0
+		return
 	}
+	// Recompute extentIdx and extentOff.  We have already
+	// established fn.fileinfo.size > ptr.off >= 0, so we don't
+	// have to deal with edge cases here.
+	var off int64
+	for ptr.extentIdx, ptr.extentOff = 0, 0; off < ptr.off; ptr.extentIdx++ {
+		// This would panic (index out of range) if
+		// fn.fileinfo.size were larger than
+		// sum(fn.extents[i].Len()) -- but that can't happen
+		// because we have ensured fn.fileinfo.size is always
+		// accurate.
+		extLen := fn.extents[ptr.extentIdx].Len()
+		if off+extLen > ptr.off {
+			ptr.extentOff = int(ptr.off - off)
+			break
+		}
+		off += extLen
+	}
+	return
 }
 
-// IsDir implements os.FileInfo.
-func (e collectionDirent) IsDir() bool {
-	return e.isDir
+func (fn *filenode) appendExtent(e extent) {
+	fn.Lock()
+	defer fn.Unlock()
+	fn.extents = append(fn.extents, e)
+	fn.fileinfo.size += e.Len()
+	fn.repacked++
 }
 
-// Size implements os.FileInfo.
-func (e collectionDirent) Size() int64 {
-	return e.size
+func (fn *filenode) OpenFile(string, int, os.FileMode) (*file, error) {
+	return nil, os.ErrNotExist
 }
 
-// Sys implements os.FileInfo.
-func (e collectionDirent) Sys() interface{} {
+func (fn *filenode) Parent() inode {
+	return fn.parent
+}
+
+func (fn *filenode) Readdir() []os.FileInfo {
 	return nil
 }
 
-// A CollectionFileSystem is an http.Filesystem with an added Stat() method.
-type CollectionFileSystem interface {
-	http.FileSystem
-	Stat(name string) (os.FileInfo, error)
+func (fn *filenode) Read(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr, err error) {
+	fn.RLock()
+	defer fn.RUnlock()
+	ptr = fn.seek(startPtr)
+	if ptr.off < 0 {
+		err = ErrNegativeOffset
+		return
+	}
+	if ptr.extentIdx >= len(fn.extents) {
+		err = io.EOF
+		return
+	}
+	n, err = fn.extents[ptr.extentIdx].ReadAt(p, int64(ptr.extentOff))
+	if n > 0 {
+		ptr.off += int64(n)
+		ptr.extentOff += n
+		if int64(ptr.extentOff) == fn.extents[ptr.extentIdx].Len() {
+			ptr.extentIdx++
+			ptr.extentOff = 0
+			if ptr.extentIdx < len(fn.extents) && err == io.EOF {
+				err = nil
+			}
+		}
+	}
+	return
 }
 
-// collectionFS implements CollectionFileSystem.
-type collectionFS struct {
-	collection *Collection
-	client     *Client
-	kc         keepClient
-	sizes      map[string]int64
-	sizesOnce  sync.Once
+func (fn *filenode) Write(p []byte, startPtr filenodePtr) (n int, ptr filenodePtr, err error) {
+	fn.Lock()
+	defer fn.Unlock()
+	ptr = fn.seek(startPtr)
+	if ptr.off < 0 {
+		err = ErrNegativeOffset
+		return
+	}
+	err = ErrReadOnlyFile
+	return
 }
 
 // FileSystem returns a CollectionFileSystem for the collection.
 func (c *Collection) FileSystem(client *Client, kc keepClient) CollectionFileSystem {
-	return &collectionFS{
-		collection: c,
-		client:     client,
-		kc:         kc,
-	}
-}
-
-func (c *collectionFS) Stat(name string) (os.FileInfo, error) {
-	name = canonicalName(name)
-	if name == "." {
-		return collectionDirent{
-			collection: c.collection,
-			name:       "/",
-			isDir:      true,
-		}, nil
-	}
-	if size, ok := c.fileSizes()[name]; ok {
-		return collectionDirent{
-			collection: c.collection,
-			name:       path.Base(name),
-			size:       size,
-			isDir:      false,
-		}, nil
-	}
-	for fnm := range c.fileSizes() {
-		if !strings.HasPrefix(fnm, name+"/") {
-			continue
+	fs := &fileSystem{dirnode: dirnode{
+		cache:    &keepBlockCache{kc: kc},
+		client:   client,
+		kc:       kc,
+		fileinfo: fileinfo{name: ".", mode: os.ModeDir | 0755},
+		parent:   nil,
+		inodes:   make(map[string]inode),
+	}}
+	fs.dirnode.parent = &fs.dirnode
+	fs.dirnode.loadManifest(c.ManifestText)
+	return fs
+}
+
+type file struct {
+	inode
+	ptr        filenodePtr
+	append     bool
+	writable   bool
+	unreaddirs []os.FileInfo
+}
+
+func (f *file) Read(p []byte) (n int, err error) {
+	n, f.ptr, err = f.inode.Read(p, f.ptr)
+	return
+}
+
+func (f *file) Seek(off int64, whence int) (pos int64, err error) {
+	size := f.inode.Size()
+	ptr := f.ptr
+	switch whence {
+	case os.SEEK_SET:
+		ptr.off = off
+	case os.SEEK_CUR:
+		ptr.off += off
+	case os.SEEK_END:
+		ptr.off = size + off
+	}
+	if ptr.off < 0 {
+		return f.ptr.off, ErrNegativeOffset
+	}
+	if ptr.off > size {
+		ptr.off = size
+	}
+	if ptr.off != f.ptr.off {
+		f.ptr = ptr
+		// force filenode to recompute f.ptr fields on next
+		// use
+		f.ptr.repacked = -1
+	}
+	return f.ptr.off, nil
+}
+
+func (f *file) Write(p []byte) (n int, err error) {
+	n, f.ptr, err = f.inode.Write(p, f.ptr)
+	return
+}
+
+func (f *file) Readdir(count int) ([]os.FileInfo, error) {
+	if !f.inode.IsDir() {
+		return nil, ErrInvalidOperation
+	}
+	if count <= 0 {
+		return f.inode.Readdir(), nil
+	}
+	if f.unreaddirs == nil {
+		f.unreaddirs = f.inode.Readdir()
+	}
+	if len(f.unreaddirs) == 0 {
+		return nil, io.EOF
+	}
+	if count > len(f.unreaddirs) {
+		count = len(f.unreaddirs)
+	}
+	ret := f.unreaddirs[:count]
+	f.unreaddirs = f.unreaddirs[count:]
+	return ret, nil
+}
+
+func (f *file) Stat() (os.FileInfo, error) {
+	return f.inode, nil
+}
+
+func (f *file) Close() error {
+	// FIXME: flush
+	return nil
+}
+
+func (f *file) OpenFile(name string, flag int, perm os.FileMode) (*file, error) {
+	return f.inode.OpenFile(name, flag, perm)
+}
+
+type dirnode struct {
+	fileinfo
+	parent *dirnode
+	client *Client
+	kc     keepClient
+	cache  blockCache
+	inodes map[string]inode
+	sync.RWMutex
+}
+
+func (dn *dirnode) loadManifest(txt string) {
+	// FIXME: faster
+	var dirname string
+	for _, stream := range strings.Split(txt, "\n") {
+		var extents []storedExtent
+		for i, token := range strings.Split(stream, " ") {
+			if i == 0 {
+				dirname = manifestUnescape(token)
+				continue
+			}
+			if !strings.Contains(token, ":") {
+				toks := strings.SplitN(token, "+", 3)
+				if len(toks) < 2 {
+					// FIXME: broken
+					continue
+				}
+				length, err := strconv.ParseInt(toks[1], 10, 32)
+				if err != nil || length < 0 {
+					// FIXME: broken
+					continue
+				}
+				extents = append(extents, storedExtent{
+					locator: token,
+					offset:  0,
+					length:  int(length),
+				})
+				continue
+			}
+			toks := strings.Split(token, ":")
+			if len(toks) != 3 {
+				// FIXME: broken manifest
+				continue
+			}
+			offset, err := strconv.ParseInt(toks[0], 10, 64)
+			if err != nil || offset < 0 {
+				// FIXME: broken manifest
+				continue
+			}
+			length, err := strconv.ParseInt(toks[1], 10, 64)
+			if err != nil || length < 0 {
+				// FIXME: broken manifest
+				continue
+			}
+			name := path.Clean(dirname + "/" + manifestUnescape(toks[2]))
+			dn.makeParentDirs(name)
+			f, err := dn.OpenFile(name, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0700)
+			if err != nil {
+				// FIXME: broken
+				continue
+			}
+			if f.inode.Stat().IsDir() {
+				f.Close()
+				// FIXME: broken manifest
+				continue
+			}
+			var pos int64
+			for _, e := range extents {
+				if pos+e.Len() < offset {
+					pos += e.Len()
+					continue
+				}
+				if pos > offset+length {
+					break
+				}
+				var blkOff int
+				if pos < offset {
+					blkOff = int(offset - pos)
+				}
+				blkLen := int(e.Len()) - blkOff
+				if pos+int64(blkOff+blkLen) > offset+length {
+					blkLen = int(offset + length - pos - int64(blkOff))
+				}
+				f.inode.(*filenode).appendExtent(storedExtent{
+					cache:   dn.cache,
+					locator: e.locator,
+					offset:  blkOff,
+					length:  blkLen,
+				})
+				pos += e.Len()
+			}
+			f.Close()
 		}
-		return collectionDirent{
-			collection: c.collection,
-			name:       path.Base(name),
-			isDir:      true,
-		}, nil
 	}
-	return nil, os.ErrNotExist
 }
 
-func (c *collectionFS) Open(name string) (http.File, error) {
-	// Ensure name looks the way it does in a manifest.
-	name = canonicalName(name)
+func (dn *dirnode) makeParentDirs(name string) {
+	names := strings.Split(name, "/")
+	for _, name := range names[:len(names)-1] {
+		dn.Lock()
+		defer dn.Unlock()
+		if n, ok := dn.inodes[name]; !ok {
+			n := &dirnode{
+				parent: dn,
+				client: dn.client,
+				kc:     dn.kc,
+				fileinfo: fileinfo{
+					name: name,
+					mode: os.ModeDir | 0755,
+				},
+			}
+			if dn.inodes == nil {
+				dn.inodes = make(map[string]inode)
+			}
+			dn.inodes[name] = n
+			dn.fileinfo.size++
+			dn = n
+		} else if n, ok := n.(*dirnode); ok {
+			dn = n
+		} else {
+			// fail
+			return
+		}
+	}
+}
+
+func (dn *dirnode) Parent() inode {
+	return dn.parent
+}
+
+func (dn *dirnode) Readdir() (fi []os.FileInfo) {
+	dn.RLock()
+	defer dn.RUnlock()
+	fi = make([]os.FileInfo, 0, len(dn.inodes))
+	for _, inode := range dn.inodes {
+		fi = append(fi, inode.Stat())
+	}
+	return
+}
 
-	m := manifest.Manifest{Text: c.collection.ManifestText}
+func (dn *dirnode) Read(p []byte, ptr filenodePtr) (int, filenodePtr, error) {
+	return 0, ptr, ErrInvalidOperation
+}
+
+func (dn *dirnode) Write(p []byte, ptr filenodePtr) (int, filenodePtr, error) {
+	return 0, ptr, ErrInvalidOperation
+}
 
-	// Return a file if it exists.
-	if size, ok := c.fileSizes()[name]; ok {
-		reader, err := c.kc.ManifestFileReader(m, name)
+func (dn *dirnode) OpenFile(name string, flag int, perm os.FileMode) (*file, error) {
+	name = strings.TrimSuffix(name, "/")
+	if name == "." || name == "" {
+		return &file{inode: dn}, nil
+	}
+	if dirname, name := path.Split(name); dirname != "" {
+		// OpenFile("foo/bar/baz") =>
+		// OpenFile("foo/bar").OpenFile("baz") (or
+		// ErrNotExist, if foo/bar is a file)
+		f, err := dn.OpenFile(dirname, os.O_RDONLY, 0)
 		if err != nil {
 			return nil, err
 		}
-		return &collectionFile{
-			File:       reader,
-			collection: c.collection,
-			name:       path.Base(name),
-			size:       size,
-		}, nil
-	}
-
-	// Return a directory if it's the root dir or there are file
-	// entries below it.
-	children := map[string]collectionDirent{}
-	for fnm, size := range c.fileSizes() {
-		if !strings.HasPrefix(fnm, name+"/") {
-			continue
+		defer f.Close()
+		if dn, ok := f.inode.(*dirnode); ok {
+			return dn.OpenFile(name, flag, perm)
+		} else {
+			return nil, os.ErrNotExist
+		}
+	}
+	dn.Lock()
+	defer dn.Unlock()
+	if name == ".." {
+		return &file{inode: dn.parent}, nil
+	}
+	n, ok := dn.inodes[name]
+	if !ok {
+		if flag&os.O_CREATE == 0 {
+			return nil, os.ErrNotExist
+		}
+		n = &filenode{
+			parent: dn,
+			fileinfo: fileinfo{
+				name: name,
+				mode: 0755,
+			},
 		}
-		isDir := false
-		ent := fnm[len(name)+1:]
-		if i := strings.Index(ent, "/"); i >= 0 {
-			ent = ent[:i]
-			isDir = true
+		if dn.inodes == nil {
+			dn.inodes = make(map[string]inode)
 		}
-		e := children[ent]
-		e.collection = c.collection
-		e.isDir = isDir
-		e.name = ent
-		e.size = size
-		children[ent] = e
-	}
-	if len(children) == 0 && name != "." {
-		return nil, os.ErrNotExist
-	}
-	dirents := make([]os.FileInfo, 0, len(children))
-	for _, ent := range children {
-		dirents = append(dirents, ent)
-	}
-	return &collectionDir{
-		collection: c.collection,
-		stream:     name,
-		dirents:    dirents,
+		dn.inodes[name] = n
+		dn.fileinfo.size++
+	} else if flag&os.O_EXCL != 0 {
+		return nil, ErrFileExists
+	}
+	return &file{
+		inode:    n,
+		append:   flag&os.O_APPEND != 0,
+		writable: flag&(os.O_WRONLY|os.O_RDWR) != 0,
 	}, nil
 }
 
-// fileSizes returns a map of files that can be opened. Each key
-// starts with "./".
-func (c *collectionFS) fileSizes() map[string]int64 {
-	c.sizesOnce.Do(func() {
-		c.sizes = map[string]int64{}
-		m := manifest.Manifest{Text: c.collection.ManifestText}
-		for ms := range m.StreamIter() {
-			for _, fss := range ms.FileStreamSegments {
-				c.sizes[ms.StreamName+"/"+fss.Name] += int64(fss.SegLen)
-			}
+type extent interface {
+	io.ReaderAt
+	Len() int64
+}
+
+type writableExtent interface {
+	extent
+	WriteAt(p []byte, off int)
+	Truncate(n int)
+}
+
+type memExtent struct {
+	buf []byte
+}
+
+func (me *memExtent) Len() int64 {
+	return int64(len(me.buf))
+}
+
+func (me *memExtent) Truncate(n int) {
+	if n > cap(me.buf) {
+		newsize := 1024
+		for newsize < n {
+			newsize = newsize << 2
+		}
+		newbuf := make([]byte, n, newsize)
+		copy(newbuf, me.buf)
+		me.buf = newbuf
+	}
+	me.buf = me.buf[:n]
+}
+
+func (me *memExtent) WriteAt(p []byte, off int) {
+	if off+len(p) > len(me.buf) {
+		panic("overflowed extent")
+	}
+	copy(me.buf[off:], p)
+}
+
+func (me *memExtent) ReadAt(p []byte, off int64) (n int, err error) {
+	if off > me.Len() {
+		err = io.EOF
+		return
+	}
+	n = copy(p, me.buf[int(off):])
+	if n < len(p) {
+		err = io.EOF
+	}
+	return
+}
+
+type storedExtent struct {
+	cache   blockCache
+	locator string
+	offset  int
+	length  int
+}
+
+func (se storedExtent) Len() int64 {
+	return int64(se.length)
+}
+
+func (se storedExtent) ReadAt(p []byte, off int64) (n int, err error) {
+	maxlen := int(int64(se.length) - int64(se.offset) - off)
+	if len(p) > maxlen {
+		p = p[:maxlen]
+		n, err = se.cache.ReadAt(se.locator, p, off+int64(se.offset))
+		if err == nil {
+			err = io.EOF
 		}
-	})
-	return c.sizes
+		return
+	}
+	return se.cache.ReadAt(se.locator, p, off+int64(se.offset))
+}
+
+type blockCache interface {
+	ReadAt(locator string, p []byte, off int64) (n int, err error)
+}
+
+type keepBlockCache struct {
+	kc keepClient
+}
+
+var scratch = make([]byte, 2<<26)
+
+func (kbc *keepBlockCache) ReadAt(locator string, p []byte, off int64) (int, error) {
+	return kbc.kc.ReadAt(locator, p, off)
 }
 
 func canonicalName(name string) string {
@@ -278,3 +657,21 @@ func canonicalName(name string) string {
 	}
 	return name
 }
+
+var manifestEscapeSeq = regexp.MustCompile(`\\([0-9]{3}|\\)`)
+
+func manifestUnescapeSeq(seq string) string {
+	if seq == `\\` {
+		return `\`
+	}
+	i, err := strconv.ParseUint(seq[1:], 8, 8)
+	if err != nil {
+		// Invalid escape sequence: can't unescape.
+		return seq
+	}
+	return string([]byte{byte(i)})
+}
+
+func manifestUnescape(s string) string {
+	return manifestEscapeSeq.ReplaceAllStringFunc(s, manifestUnescapeSeq)
+}
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index f51d1eb..7bd3570 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -16,17 +16,39 @@ import (
 
 var _ = check.Suite(&CollectionFSSuite{})
 
+type keepClientStub struct {
+	blocks map[string][]byte
+}
+
+func (kcs *keepClientStub) ReadAt(locator string, p []byte, off int64) (int, error) {
+	buf := kcs.blocks[locator[:32]]
+	if buf == nil {
+		return 0, os.ErrNotExist
+	}
+	return copy(p, buf[int(off):]), nil
+}
+
 type CollectionFSSuite struct {
 	client *Client
 	coll   Collection
-	fs     http.FileSystem
+	fs     CollectionFileSystem
+	kc     keepClient
 }
 
 func (s *CollectionFSSuite) SetUpTest(c *check.C) {
 	s.client = NewClientFromEnv()
 	err := s.client.RequestAndDecode(&s.coll, "GET", "arvados/v1/collections/"+arvadostest.FooAndBarFilesInDirUUID, nil, nil)
 	c.Assert(err, check.IsNil)
-	s.fs = s.coll.FileSystem(s.client, nil)
+	s.kc = &keepClientStub{
+		blocks: map[string][]byte{
+			"3858f62230ac3c915f300c664312c63f": []byte("foobar"),
+		}}
+	s.fs = s.coll.FileSystem(s.client, s.kc)
+}
+
+func (s *CollectionFSSuite) TestHttpFileSystemInterface(c *check.C) {
+	_, ok := s.fs.(http.FileSystem)
+	c.Check(ok, check.Equals, true)
 }
 
 func (s *CollectionFSSuite) TestReaddirFull(c *check.C) {
@@ -58,7 +80,7 @@ func (s *CollectionFSSuite) TestReaddirLimited(c *check.C) {
 	}
 
 	fis, err = f.Readdir(1)
-	c.Check(err, check.Equals, io.EOF)
+	c.Check(err, check.IsNil)
 	c.Check(len(fis), check.Equals, 1)
 	if len(fis) > 0 {
 		c.Check(fis[0].Size(), check.Equals, int64(3))
@@ -76,7 +98,7 @@ func (s *CollectionFSSuite) TestReaddirLimited(c *check.C) {
 	c.Assert(err, check.IsNil)
 	fis, err = f.Readdir(2)
 	c.Check(len(fis), check.Equals, 1)
-	c.Assert(err, check.Equals, io.EOF)
+	c.Assert(err, check.IsNil)
 	fis, err = f.Readdir(2)
 	c.Check(len(fis), check.Equals, 0)
 	c.Assert(err, check.Equals, io.EOF)
diff --git a/sdk/go/keepclient/block_cache.go b/sdk/go/keepclient/block_cache.go
index e841a00..c1138fa 100644
--- a/sdk/go/keepclient/block_cache.go
+++ b/sdk/go/keepclient/block_cache.go
@@ -49,6 +49,19 @@ func (c *BlockCache) Sweep() {
 	}
 }
 
+// ReadAt returns data from the cache, first retrieving it from Keep if
+// necessary.
+func (c *BlockCache) ReadAt(kc *KeepClient, locator string, p []byte, off int64) (int, error) {
+	buf, err := c.Get(kc, locator)
+	if err != nil {
+		return 0, err
+	}
+	if off > int64(len(buf)) {
+		return 0, io.ErrUnexpectedEOF
+	}
+	return copy(p, buf[int(off):]), nil
+}
+
 // Get returns data from the cache, first retrieving it from Keep if
 // necessary.
 func (c *BlockCache) Get(kc *KeepClient, locator string) ([]byte, error) {
diff --git a/sdk/go/keepclient/collectionreader.go b/sdk/go/keepclient/collectionreader.go
index 57829aa..3f39aff 100644
--- a/sdk/go/keepclient/collectionreader.go
+++ b/sdk/go/keepclient/collectionreader.go
@@ -43,14 +43,7 @@ func (kc *KeepClient) CollectionFileReader(collection map[string]interface{}, fi
 }
 
 func (kc *KeepClient) ManifestFileReader(m manifest.Manifest, filename string) (arvados.File, error) {
-	f := &file{
-		kc: kc,
-	}
-	err := f.load(m, filename)
-	if err != nil {
-		return nil, err
-	}
-	return f, nil
+	return (&arvados.Collection{ManifestText: m.Text}).FileSystem(nil, kc).OpenFile(filename, os.O_RDONLY, 0)
 }
 
 type file struct {
diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index cbfad81..c3d63ed 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -292,6 +292,12 @@ func (kc *KeepClient) Get(locator string) (io.ReadCloser, int64, string, error)
 	return kc.getOrHead("GET", locator)
 }
 
+// ReadAt() retrieves a portion of block from the cache if it's
+// present, otherwise from the network.
+func (kc *KeepClient) ReadAt(locator string, p []byte, off int64) (int, error) {
+	return kc.cache().ReadAt(kc, locator, p, off)
+}
+
 // Ask() verifies that a block with the given hash is available and
 // readable, according to at least one Keep service. Unlike Get, it
 // does not retrieve the data or verify that the data content matches

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list