X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/9b9ada224856e289cdd9e81954c4ea3c3bc1fe68..6fa7f9fbcf20aa866eed0618bd09e1ce2e109baa:/services/keep-web/handler.go

diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 0df19d443d..b9250efec7 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -27,15 +27,14 @@ import (
 	"git.arvados.org/arvados.git/sdk/go/auth"
 	"git.arvados.org/arvados.git/sdk/go/ctxlog"
 	"git.arvados.org/arvados.git/sdk/go/httpserver"
-	"git.arvados.org/arvados.git/sdk/go/keepclient"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/net/webdav"
 )
 
 type handler struct {
-	Cache     cache
-	Cluster   *arvados.Cluster
-	setupOnce sync.Once
+	Cache   cache
+	Cluster *arvados.Cluster
+	metrics *metrics
 
 	lockMtx    sync.Mutex
 	lock       map[string]*sync.RWMutex
@@ -60,10 +59,6 @@ func parseCollectionIDFromURL(s string) string {
 	return ""
 }
 
-func (h *handler) setup() {
-	keepclient.DefaultBlockCache.MaxBlocks = h.Cluster.Collections.WebDAVCache.MaxBlockEntries
-}
-
 func (h *handler) serveStatus(w http.ResponseWriter, r *http.Request) {
 	json.NewEncoder(w).Encode(struct{ Version string }{cmd.Version.String()})
 }
@@ -179,13 +174,16 @@ func (h *handler) Done() <-chan struct{} {
 
 // ServeHTTP implements http.Handler.
 func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
-	h.setupOnce.Do(h.setup)
-
 	if xfp := r.Header.Get("X-Forwarded-Proto"); xfp != "" && xfp != "http" {
 		r.URL.Scheme = xfp
 	}
 
-	w := httpserver.WrapResponseWriter(wOrig)
+	wbuffer := newWriteBuffer(wOrig, int(h.Cluster.Collections.WebDAVOutputBuffer))
+	defer wbuffer.Close()
+	w := httpserver.WrapResponseWriter(responseWriter{
+		Writer:         wbuffer,
+		ResponseWriter: wOrig,
+	})
 
 	if r.Method == "OPTIONS" && ServeCORSPreflight(w, r.Header) {
 		return
@@ -297,10 +295,12 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	origin := r.Header.Get("Origin")
 	cors := origin != "" && !strings.HasSuffix(origin, "://"+r.Host)
 	safeAjax := cors && (r.Method == http.MethodGet || r.Method == http.MethodHead)
-	// Important distiction: safeAttachment checks whether api_token exists as
-	// a query parameter. The following condition checks whether api_token
-	// exists as request form data *or* a query parameter. This distinction is
-	// necessary to redirect when required, and not when not.
+	// Important distinction: safeAttachment checks whether api_token exists
+	// as a query parameter. haveFormTokens checks whether api_token exists
+	// as request form data *or* a query parameter. Different checks are
+	// necessary because both the request disposition and the location of
+	// the API token affect whether or not the request needs to be
+	// redirected. The different branch comments below explain further.
 	safeAttachment := attachment && !r.URL.Query().Has("api_token")
 	if formTokens, haveFormTokens := r.Form["api_token"]; !haveFormTokens {
 		// No token to use or redact.
@@ -430,11 +430,26 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 			return
 		}
 		defer f.Close()
-		defer sess.Release()
 
 		collectionDir, sessionFS, session, tokenUser = f, fs, sess, user
 		break
 	}
+
+	// releaseSession() is equivalent to session.Release() except
+	// that it's a no-op if (1) session is nil, or (2) it has
+	// already been called.
+	//
+	// This way, we can do a defer call here to ensure it gets
+	// called in all code paths, and also call it inline (see
+	// below) in the cases where we want to release the lock
+	// before returning.
+	releaseSession := func() {}
+	if session != nil {
+		var releaseSessionOnce sync.Once
+		releaseSession = func() { releaseSessionOnce.Do(func() { session.Release() }) }
+	}
+	defer releaseSession()
+
 	if forceReload && collectionDir != nil {
 		err := collectionDir.Sync()
 		if err != nil {
@@ -522,6 +537,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	if r.Method == http.MethodGet || r.Method == http.MethodHead {
 		targetfnm := fsprefix + strings.Join(pathParts[stripParts:], "/")
 		if fi, err := sessionFS.Stat(targetfnm); err == nil && fi.IsDir() {
+			releaseSession() // because we won't be writing anything
 			if !strings.HasSuffix(r.URL.Path, "/") {
 				h.seeOtherWithCookie(w, r, r.URL.Path+"/", credentialsOK)
 			} else {
@@ -591,6 +607,15 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 				collectionDir.Splice(snap)
 				return nil
 			}}
+	} else {
+		// When writing, we need to block session renewal
+		// until we're finished, in order to guarantee the
+		// effect of the write is visible in future responses.
+		// But if we're not writing, we can release the lock
+		// early.  This enables us to keep renewing sessions
+		// and processing more requests even if a slow client
+		// takes a long time to download a large file.
+		releaseSession()
 	}
 	if r.Method == http.MethodGet {
 		applyContentDispositionHdr(w, r, basename, attachment)
@@ -598,7 +623,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	if webdavPrefix == "" {
 		webdavPrefix = "/" + strings.Join(pathParts[:stripParts], "/")
 	}
-	wh := webdav.Handler{
+	wh := &webdav.Handler{
 		Prefix: webdavPrefix,
 		FileSystem: &webdavfs.FS{
 			FileSystem:    sessionFS,
@@ -613,7 +638,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 			}
 		},
 	}
-	wh.ServeHTTP(w, r)
+	h.metrics.track(wh, w, r)
 	if r.Method == http.MethodGet && w.WroteStatus() == http.StatusOK {
 		wrote := int64(w.WroteBodyBytes())
 		fnm := strings.Join(pathParts[stripParts:], "/")