[ARVADOS] created: 1.1.0-163-g70111d8
Git user
git at public.curoverse.com
Tue Nov 21 02:16:50 EST 2017
at 70111d82653ca633d2397c74eb994a8a9d718e80 (commit)
commit 70111d82653ca633d2397c74eb994a8a9d718e80
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Tue Nov 21 01:59:27 2017 -0500
12483: Delay flushing collection until successful http response.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>
diff --git a/sdk/go/httpserver/responsewriter.go b/sdk/go/httpserver/responsewriter.go
index e6ac4ca..3941c13 100644
--- a/sdk/go/httpserver/responsewriter.go
+++ b/sdk/go/httpserver/responsewriter.go
@@ -8,40 +8,46 @@ import (
"net/http"
)
-// ResponseWriter wraps http.ResponseWriter and exposes the status
+type ResponseWriter interface {
+ http.ResponseWriter
+ WroteStatus() int
+ WroteBodyBytes() int
+}
+
+// responseWriter wraps http.ResponseWriter and exposes the status
// sent, the number of bytes sent to the client, and the last write
// error.
-type ResponseWriter struct {
+type responseWriter struct {
http.ResponseWriter
- wroteStatus *int // Last status given to WriteHeader()
- wroteBodyBytes *int // Bytes successfully written
- err *error // Last error returned from Write()
+ wroteStatus int // Last status given to WriteHeader()
+ wroteBodyBytes int // Bytes successfully written
+ err error // Last error returned from Write()
}
func WrapResponseWriter(orig http.ResponseWriter) ResponseWriter {
- return ResponseWriter{orig, new(int), new(int), new(error)}
+ return &responseWriter{ResponseWriter: orig}
}
-func (w ResponseWriter) WriteHeader(s int) {
- *w.wroteStatus = s
+func (w responseWriter) WriteHeader(s int) {
+ w.wroteStatus = s
w.ResponseWriter.WriteHeader(s)
}
-func (w ResponseWriter) Write(data []byte) (n int, err error) {
+func (w responseWriter) Write(data []byte) (n int, err error) {
n, err = w.ResponseWriter.Write(data)
- *w.wroteBodyBytes += n
- *w.err = err
+ w.wroteBodyBytes += n
+ w.err = err
return
}
-func (w ResponseWriter) WroteStatus() int {
- return *w.wroteStatus
+func (w responseWriter) WroteStatus() int {
+ return w.wroteStatus
}
-func (w ResponseWriter) WroteBodyBytes() int {
- return *w.wroteBodyBytes
+func (w responseWriter) WroteBodyBytes() int {
+ return w.wroteBodyBytes
}
-func (w ResponseWriter) Err() error {
- return *w.err
+func (w responseWriter) Err() error {
+ return w.err
}
diff --git a/services/arv-git-httpd/auth_handler.go b/services/arv-git-httpd/auth_handler.go
index b7373b5..617c732 100644
--- a/services/arv-git-httpd/auth_handler.go
+++ b/services/arv-git-httpd/auth_handler.go
@@ -188,5 +188,5 @@ func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
}
r.URL.Path = rewrittenPath
- h.handler.ServeHTTP(&w, r)
+ h.handler.ServeHTTP(w, r)
}
diff --git a/services/keep-web/cadaver_test.go b/services/keep-web/cadaver_test.go
index 991e923..7c05ccc 100644
--- a/services/keep-web/cadaver_test.go
+++ b/services/keep-web/cadaver_test.go
@@ -101,12 +101,8 @@ func (s *IntegrationSuite) TestWebdavWithCadaver(c *check.C) {
match: `(?ms).*Moving .* succeeded.*`,
},
{
- // Strangely, webdav deletes dst if you do
- // "move nonexistent dst" -- otherwise we
- // would repeat the above "move testfile
- // newdir0" here.
path: writePath,
- cmd: "move testfile nonexistentdir\n",
+ cmd: "move testfile newdir0\n",
match: `(?ms).*Moving .* failed.*`,
},
{
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index f10cb59..a30d40d 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -6,6 +6,7 @@ package main
import (
"encoding/json"
+ "errors"
"fmt"
"html"
"html/template"
@@ -96,6 +97,44 @@ func (h *handler) serveStatus(w http.ResponseWriter, r *http.Request) {
json.NewEncoder(w).Encode(status)
}
+// updateOnSuccess wraps httpserver.ResponseWriter. If the handler
+// sends an HTTP header indicating success, updateOnSuccess first
+// calls the provided update func. If the update func fails, a 500
+// response is sent, and the status code and body sent by the handler
+// are ignored (all response writes return errors).
+type updateOnSuccess struct {
+ httpserver.ResponseWriter
+ update func() error
+ sentHeader bool
+ dropBody bool
+}
+
+var errUpdateFailed = errors.New("update failed")
+
+func (uos *updateOnSuccess) Write(p []byte) (int, error) {
+ if uos.dropBody {
+ return 0, errUpdateFailed
+ }
+ if !uos.sentHeader {
+ uos.WriteHeader(http.StatusOK)
+ }
+ return uos.ResponseWriter.Write(p)
+}
+
+func (uos *updateOnSuccess) WriteHeader(code int) {
+ if !uos.sentHeader {
+ if code >= 200 && code < 400 {
+ if err := uos.update(); err != nil {
+ http.Error(uos.ResponseWriter, err.Error(), http.StatusInternalServerError)
+ uos.dropBody = true
+ return
+ }
+ }
+ uos.sentHeader = true
+ }
+ uos.ResponseWriter.WriteHeader(code)
+}
+
var (
webdavMethod = map[string]bool{
"DELETE": true,
@@ -368,17 +407,23 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
return
}
if webdavMethod[r.Method] {
- var update func() error
- if !arvadosclient.PDHMatch(targetID) {
- update = func() error {
- return h.Config.Cache.Update(client, *collection, fs)
- }
+ writing := !arvadosclient.PDHMatch(targetID)
+ if writing {
+ // Save the collection only if/when all
+ // webdav->filesystem operations succeed --
+ // and send a 500 error the modified
+ // collection can't be saved.
+ w = &updateOnSuccess{
+ ResponseWriter: w,
+ update: func() error {
+ return h.Config.Cache.Update(client, *collection, fs)
+ }}
}
h := webdav.Handler{
Prefix: "/" + strings.Join(pathParts[:stripParts], "/"),
FileSystem: &webdavFS{
- collfs: fs,
- update: update,
+ collfs: fs,
+ writing: writing,
},
LockSystem: h.webdavLS,
Logger: func(_ *http.Request, err error) {
diff --git a/services/keep-web/webdav.go b/services/keep-web/webdav.go
index 1a306e3..3e12f27 100644
--- a/services/keep-web/webdav.go
+++ b/services/keep-web/webdav.go
@@ -35,8 +35,8 @@ var (
// existence automatically so sequences like "mkcol foo; put foo/bar"
// work as expected.
type webdavFS struct {
- collfs arvados.CollectionFileSystem
- update func() error
+ collfs arvados.CollectionFileSystem
+ writing bool
}
func (fs *webdavFS) makeparents(name string) {
@@ -50,69 +50,45 @@ func (fs *webdavFS) makeparents(name string) {
}
func (fs *webdavFS) Mkdir(ctx context.Context, name string, perm os.FileMode) error {
- if fs.update == nil {
+ if !fs.writing {
return errReadOnly
}
name = strings.TrimRight(name, "/")
fs.makeparents(name)
- if err := fs.collfs.Mkdir(name, 0755); err != nil {
- return err
- }
- return fs.update()
+ return fs.collfs.Mkdir(name, 0755)
}
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 {
- if fs.update == nil {
+ if !fs.writing {
return nil, errReadOnly
}
fs.makeparents(name)
}
f, err = fs.collfs.OpenFile(name, flag, perm)
- if writing && err == nil {
- f = writingFile{File: f, update: fs.update}
- }
return
}
func (fs *webdavFS) RemoveAll(ctx context.Context, name string) error {
- if err := fs.collfs.RemoveAll(name); err != nil {
- return err
- }
- return fs.update()
+ return fs.collfs.RemoveAll(name)
}
func (fs *webdavFS) Rename(ctx context.Context, oldName, newName string) error {
- if fs.update == nil {
+ if !fs.writing {
return errReadOnly
}
fs.makeparents(newName)
- if err := fs.collfs.Rename(oldName, newName); err != nil {
- return err
- }
- return fs.update()
+ return fs.collfs.Rename(oldName, newName)
}
func (fs *webdavFS) Stat(ctx context.Context, name string) (os.FileInfo, error) {
- if fs.update != nil {
+ if fs.writing {
fs.makeparents(name)
}
return fs.collfs.Stat(name)
}
-type writingFile struct {
- webdav.File
- update func() error
-}
-
-func (f writingFile) Close() error {
- if err := f.File.Close(); err != nil || f.update == nil {
- return err
- }
- return f.update()
-}
-
// noLockSystem implements webdav.LockSystem by returning success for
// every possible locking operation, even though it has no side
// effects such as actually locking anything. This works for a
commit 11127c5b67a10a46f60c1c1c53a2c2639b7914e1
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date: Mon Nov 20 18:43:46 2017 -0500
12483: Make webdav rename/remove work. Tidy up 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 4c97af5..3f5fb5e 100644
--- a/sdk/go/arvados/collection_fs.go
+++ b/sdk/go/arvados/collection_fs.go
@@ -735,8 +735,7 @@ func (dn *dirnode) marshalManifest(prefix string) (string, error) {
sort.Strings(names)
for _, name := range names {
- node := dn.inodes[name]
- switch node := node.(type) {
+ switch node := dn.inodes[name].(type) {
case *dirnode:
subdir, err := node.marshalManifest(prefix + "/" + name)
if err != nil {
@@ -793,14 +792,14 @@ func (dn *dirnode) marshalManifest(prefix string) (string, error) {
}
func (dn *dirnode) loadManifest(txt string) error {
- // FIXME: faster
var dirname string
streams := strings.Split(txt, "\n")
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] {
+ streams = streams[:len(streams)-1]
+ extents := []storedExtent{}
+ for i, stream := range streams {
lineno := i + 1
var anyFileTokens bool
var pos int64
@@ -863,7 +862,7 @@ func (dn *dirnode) loadManifest(txt string) error {
// situation might be rare anyway)
extIdx, pos = 0, 0
}
- for next := int64(0); extIdx < len(extents); extIdx, pos = extIdx+1, next {
+ for next := int64(0); extIdx < len(extents); extIdx++ {
e := extents[extIdx]
next = pos + int64(e.Len())
if next <= offset || e.Len() == 0 {
@@ -890,6 +889,8 @@ func (dn *dirnode) loadManifest(txt string) error {
})
if next > offset+length {
break
+ } else {
+ pos = next
}
}
if extIdx == len(extents) && pos < offset+length {
@@ -910,36 +911,35 @@ func (dn *dirnode) loadManifest(txt string) error {
// 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 == ".." {
+ basename := names[len(names)-1]
+ if 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
- }
- if name == "" || name == "." {
- continue
- }
- if name == ".." {
- node = dn.parent
- continue
- }
- node, ok = dn.inodes[name]
- if !ok {
- if i == len(names)-1 {
- fn = dn.newFilenode(name, 0755)
+ for _, name := range names[:len(names)-1] {
+ switch name {
+ case "", ".":
+ case "..":
+ dn = dn.parent
+ default:
+ switch node := dn.inodes[name].(type) {
+ case nil:
+ dn = dn.newDirnode(name, 0755)
+ case *dirnode:
+ dn = node
+ case *filenode:
+ err = ErrFileExists
return
}
- node = dn.newDirnode(name, 0755)
}
}
- var ok bool
- if fn, ok = node.(*filenode); !ok {
- err = ErrInvalidArgument
+ switch node := dn.inodes[basename].(type) {
+ case nil:
+ fn = dn.newFilenode(basename, 0755)
+ case *filenode:
+ fn = node
+ case *dirnode:
+ err = ErrIsDirectory
}
return
}
@@ -957,11 +957,17 @@ func (dn *dirnode) Mkdir(name string, perm os.FileMode) error {
}
func (dn *dirnode) Remove(name string) error {
- return dn.remove(name, false)
+ return dn.remove(strings.TrimRight(name, "/"), false)
}
func (dn *dirnode) RemoveAll(name string) error {
- return dn.remove(name, true)
+ err := dn.remove(strings.TrimRight(name, "/"), true)
+ if os.IsNotExist(err) {
+ // "If the path does not exist, RemoveAll returns
+ // nil." (see "os" pkg)
+ err = nil
+ }
+ return err
}
func (dn *dirnode) remove(name string, recursive bool) error {
@@ -1054,9 +1060,20 @@ func (dn *dirnode) Rename(oldname, newname string) error {
}
}
} else {
+ if newdn.inodes == nil {
+ newdn.inodes = make(map[string]inode)
+ }
newdn.fileinfo.size++
}
newdn.inodes[newname] = oldinode
+ switch n := oldinode.(type) {
+ case *dirnode:
+ n.parent = newdn
+ case *filenode:
+ n.parent = newdn
+ default:
+ panic(fmt.Sprintf("bad inode type %T", n))
+ }
delete(olddn.inodes, oldname)
olddn.fileinfo.size--
return nil
diff --git a/sdk/go/arvados/collection_fs_test.go b/sdk/go/arvados/collection_fs_test.go
index 07d00be..dabb884 100644
--- a/sdk/go/arvados/collection_fs_test.go
+++ b/sdk/go/arvados/collection_fs_test.go
@@ -560,14 +560,14 @@ func (s *CollectionFSSuite) TestRemove(c *check.C) {
c.Assert(err, check.IsNil)
err = fs.Mkdir("dir1/dir2", 0755)
c.Assert(err, check.IsNil)
+ err = fs.Mkdir("dir1/dir3", 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/..")
@@ -576,10 +576,12 @@ func (s *CollectionFSSuite) TestRemove(c *check.C) {
c.Check(err, check.Equals, ErrDirectoryNotEmpty)
err = fs.Remove("dir1/dir2/../../../dir1")
c.Check(err, check.Equals, ErrDirectoryNotEmpty)
+ err = fs.Remove("dir1/dir3/")
+ c.Check(err, check.IsNil)
err = fs.RemoveAll("dir1")
c.Check(err, check.IsNil)
err = fs.RemoveAll("dir1")
- c.Check(err, check.Equals, os.ErrNotExist)
+ c.Check(err, check.IsNil)
}
func (s *CollectionFSSuite) TestRename(c *check.C) {
@@ -630,7 +632,7 @@ func (s *CollectionFSSuite) TestRename(c *check.C) {
// oldname does not exist
err := fs.Rename(
fmt.Sprintf("dir%d/dir%d/missing", i, j),
- fmt.Sprintf("dir%d/irelevant", outer-i-1))
+ fmt.Sprintf("dir%d/dir%d/file%d", outer-i-1, j, j))
c.Check(err, check.ErrorMatches, `.*does not exist`)
// newname parent dir does not exist
diff --git a/services/keep-web/cadaver_test.go b/services/keep-web/cadaver_test.go
index eab4a70..991e923 100644
--- a/services/keep-web/cadaver_test.go
+++ b/services/keep-web/cadaver_test.go
@@ -95,6 +95,71 @@ func (s *IntegrationSuite) TestWebdavWithCadaver(c *check.C) {
match: `(?ms).*succeeded.*`,
data: testdata,
},
+ {
+ path: writePath,
+ cmd: "move testfile newdir0\n",
+ match: `(?ms).*Moving .* succeeded.*`,
+ },
+ {
+ // Strangely, webdav deletes dst if you do
+ // "move nonexistent dst" -- otherwise we
+ // would repeat the above "move testfile
+ // newdir0" here.
+ path: writePath,
+ cmd: "move testfile nonexistentdir\n",
+ match: `(?ms).*Moving .* failed.*`,
+ },
+ {
+ path: writePath,
+ cmd: "ls\n",
+ match: `(?ms).*newdir0.* 0 .*`,
+ },
+ {
+ path: writePath,
+ cmd: "move newdir0/testfile emptyfile/bogus/\n",
+ match: `(?ms).*Moving .* failed.*`,
+ },
+ {
+ path: writePath,
+ cmd: "mkcol newdir1\n",
+ match: `(?ms).*Creating .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "move newdir0/testfile newdir1\n",
+ match: `(?ms).*Moving .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "put '" + localfile.Name() + "' newdir1/testfile1\n",
+ match: `(?ms).*Uploading .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "mkcol newdir2\n",
+ match: `(?ms).*Creating .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "put '" + localfile.Name() + "' newdir2/testfile2\n",
+ match: `(?ms).*Uploading .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "get newdir2/testfile2 '" + checkfile.Name() + "'\n",
+ match: `(?ms).*succeeded.*`,
+ data: testdata,
+ },
+ {
+ path: writePath,
+ cmd: "rmcol newdir2\n",
+ match: `(?ms).*Deleting collection .* succeeded.*`,
+ },
+ {
+ path: writePath,
+ cmd: "get newdir2/testfile2 '" + checkfile.Name() + "'\n",
+ match: `(?ms).*Downloading .* failed.*`,
+ },
} {
c.Logf("%s %+v", "http://"+s.testServer.Addr, trial)
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index f6c12c3..f10cb59 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -98,10 +98,13 @@ func (h *handler) serveStatus(w http.ResponseWriter, r *http.Request) {
var (
webdavMethod = map[string]bool{
+ "DELETE": true,
+ "MKCOL": true,
"MOVE": true,
"OPTIONS": true,
"PROPFIND": true,
"PUT": true,
+ "RMCOL": true,
}
browserMethod = map[string]bool{
"GET": true,
@@ -149,7 +152,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, MOVE, OPTIONS, POST, PROPFIND, PUT")
+ w.Header().Set("Access-Control-Allow-Methods", "DELETE, GET, MKCOL, MOVE, OPTIONS, POST, PROPFIND, PUT, RMCOL")
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 6add804..85a8e50 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -45,12 +45,12 @@ 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, MOVE, OPTIONS, POST, PROPFIND, PUT")
+ c.Check(resp.Header().Get("Access-Control-Allow-Methods"), check.Equals, "DELETE, GET, MKCOL, MOVE, OPTIONS, POST, PROPFIND, PUT, RMCOL")
c.Check(resp.Header().Get("Access-Control-Allow-Headers"), check.Equals, "Authorization, Content-Type, Range")
// Check preflight for a disallowed request
resp = httptest.NewRecorder()
- req.Header.Set("Access-Control-Request-Method", "DELETE")
+ req.Header.Set("Access-Control-Request-Method", "MAKE-COFFEE")
h.ServeHTTP(resp, req)
c.Check(resp.Body.String(), check.Equals, "")
c.Check(resp.Code, check.Equals, http.StatusMethodNotAllowed)
diff --git a/services/keep-web/webdav.go b/services/keep-web/webdav.go
index 7798a69..1a306e3 100644
--- a/services/keep-web/webdav.go
+++ b/services/keep-web/webdav.go
@@ -10,6 +10,8 @@ import (
"fmt"
prand "math/rand"
"os"
+ "path"
+ "strings"
"sync/atomic"
"time"
@@ -27,19 +29,45 @@ var (
// webdavFS implements a webdav.FileSystem by wrapping an
// arvados.CollectionFilesystem.
+//
+// Collections don't preserve empty directories, so Mkdir is
+// effectively a no-op, and we need to make parent dirs spring into
+// existence automatically so sequences like "mkcol foo; put foo/bar"
+// work as expected.
type webdavFS struct {
collfs arvados.CollectionFileSystem
update func() error
}
+func (fs *webdavFS) makeparents(name string) {
+ dir, name := path.Split(name)
+ if dir == "" || dir == "/" {
+ return
+ }
+ dir = dir[:len(dir)-1]
+ fs.makeparents(dir)
+ fs.collfs.Mkdir(dir, 0755)
+}
+
func (fs *webdavFS) Mkdir(ctx context.Context, name string, perm os.FileMode) error {
- return fs.collfs.Mkdir(name, 0755)
+ if fs.update == nil {
+ return errReadOnly
+ }
+ name = strings.TrimRight(name, "/")
+ fs.makeparents(name)
+ if err := fs.collfs.Mkdir(name, 0755); err != nil {
+ return err
+ }
+ return fs.update()
}
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
+ if writing {
+ if fs.update == nil {
+ return nil, errReadOnly
+ }
+ fs.makeparents(name)
}
f, err = fs.collfs.OpenFile(name, flag, perm)
if writing && err == nil {
@@ -49,17 +77,27 @@ func (fs *webdavFS) OpenFile(ctx context.Context, name string, flag int, perm os
}
func (fs *webdavFS) RemoveAll(ctx context.Context, name string) error {
- return fs.collfs.RemoveAll(name)
+ if err := fs.collfs.RemoveAll(name); err != nil {
+ return err
+ }
+ return fs.update()
}
func (fs *webdavFS) Rename(ctx context.Context, oldName, newName string) error {
if fs.update == nil {
return errReadOnly
}
- return fs.collfs.Rename(oldName, newName)
+ fs.makeparents(newName)
+ if err := fs.collfs.Rename(oldName, newName); err != nil {
+ return err
+ }
+ return fs.update()
}
func (fs *webdavFS) Stat(ctx context.Context, name string) (os.FileInfo, error) {
+ if fs.update != nil {
+ fs.makeparents(name)
+ }
return fs.collfs.Stat(name)
}
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