[arvados] created: 2.7.0-6546-gcd540d40f0

git repository hosting git at public.arvados.org
Tue May 14 00:14:15 UTC 2024


        at  cd540d40f0b38e8baaeecfc2a9ed76f2b3d2f402 (commit)


commit cd540d40f0b38e8baaeecfc2a9ed76f2b3d2f402
Author: Tom Clegg <tom at curii.com>
Date:   Mon May 13 20:13:41 2024 -0400

    21766: Enable keepclient debug logging in keep-web.
    
    Clients can now specify where to send logs by setting the Logger field
    on an arvados.Client or arvadosclient.ArvadosClient.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/sdk/go/arvados/client.go b/sdk/go/arvados/client.go
index 7bc3d5bc42..09185d1d6b 100644
--- a/sdk/go/arvados/client.go
+++ b/sdk/go/arvados/client.go
@@ -32,6 +32,7 @@ import (
 
 	"git.arvados.org/arvados.git/sdk/go/httpserver"
 	"github.com/hashicorp/go-retryablehttp"
+	"github.com/sirupsen/logrus"
 )
 
 // A Client is an HTTP client with an API endpoint and a set of
@@ -82,6 +83,9 @@ type Client struct {
 	// filesystem size.
 	DiskCacheSize ByteSizeOrPercent
 
+	// Where to write debug logs. May be nil.
+	Logger logrus.FieldLogger
+
 	dd *DiscoveryDocument
 
 	defaultRequestID string
diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index d0ebdc1b01..1f849ebace 100644
--- a/sdk/go/arvadosclient/arvadosclient.go
+++ b/sdk/go/arvadosclient/arvadosclient.go
@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"git.arvados.org/arvados.git/sdk/go/arvados"
+	"github.com/sirupsen/logrus"
 )
 
 type StringMatcher func(string) bool
@@ -110,6 +111,9 @@ type ArvadosClient struct {
 	// filesystem size.
 	DiskCacheSize arvados.ByteSizeOrPercent
 
+	// Where to write debug logs. May be nil.
+	Logger logrus.FieldLogger
+
 	// Discovery document
 	DiscoveryDoc Dict
 
@@ -150,6 +154,7 @@ func New(c *arvados.Client) (*ArvadosClient, error) {
 		Retries:           2,
 		KeepServiceURIs:   c.KeepServiceURIs,
 		DiskCacheSize:     c.DiskCacheSize,
+		Logger:            c.Logger,
 		lastClosedIdlesAt: time.Now(),
 	}
 
diff --git a/sdk/go/keepclient/discover.go b/sdk/go/keepclient/discover.go
index 5eafbbe339..192d4738e6 100644
--- a/sdk/go/keepclient/discover.go
+++ b/sdk/go/keepclient/discover.go
@@ -102,7 +102,11 @@ func (ent *cachedSvcList) poll() {
 		var next svcList
 		err := ent.arv.Call("GET", "keep_services", "", "accessible", nil, &next)
 		if err != nil {
-			log.Printf("WARNING: Error retrieving services list: %v (retrying in %v)", err, errDelay)
+			if ent.arv.Logger != nil {
+				ent.arv.Logger.WithError(err).Warnf("error retrieving services list (retrying in %v)", errDelay)
+			} else {
+				log.Printf("WARNING: Error retrieving services list: %s (retrying in %v)", err, errDelay)
+			}
 			timer.Reset(errDelay)
 			continue
 		}
diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 1720096aee..e11d68c6d1 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -211,8 +211,8 @@ func New(arv *arvadosclient.ArvadosClient) *KeepClient {
 		Retries:       2,
 	}
 	err = kc.loadDefaultClasses()
-	if err != nil {
-		DebugPrintf("DEBUG: Unable to load the default storage classes cluster config")
+	if err != nil && arv.Logger != nil {
+		arv.Logger.WithError(err).Debug("unable to load the default storage classes cluster config")
 	}
 	return kc
 }
@@ -372,7 +372,9 @@ func (kc *KeepClient) getOrHead(method string, locator string, header http.Heade
 			time.Sleep(delay.Next())
 		}
 	}
-	DebugPrintf("DEBUG: %s %s failed: %v", method, locator, errs)
+	if kc.arv.Logger != nil {
+		kc.arv.Logger.Debugf("DEBUG: %s %s failed: %v", method, locator, errs)
+	}
 
 	var err error
 	if count404 == numServers {
@@ -420,6 +422,7 @@ func (kc *KeepClient) upstreamGateway() arvados.KeepGateway {
 			Dir:         cachedir,
 			MaxSize:     kc.DiskCacheSize,
 			KeepGateway: backend,
+			Logger:      kc.Arvados.Logger,
 		}
 	}
 	return kc.gatewayStack
@@ -727,6 +730,13 @@ func (kc *KeepClient) getRequestID() string {
 	return reqIDGen.Next()
 }
 
+func (kc *KeepClient) debugf(format string, args ...interface{}) {
+	if kc.Arvados.Logger == nil {
+		return
+	}
+	kc.Arvados.Logger.Debugf(format, args...)
+}
+
 type Locator struct {
 	Hash  string
 	Size  int      // -1 if data size is not known
diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index 531db31b25..fb59460c72 100644
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -11,7 +11,6 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"log"
 	"net"
 	"net/http"
 	"os"
@@ -189,8 +188,6 @@ func UploadToStubHelper(c *C, st http.Handler, f func(*KeepClient, string,
 }
 
 func (s *StandaloneSuite) TestUploadToStubKeepServer(c *C) {
-	log.Printf("TestUploadToStubKeepServer")
-
 	st := &StubPutHandler{
 		c:                    c,
 		expectPath:           "acbd18db4cc2f85cedef654fccc4a4d8",
diff --git a/sdk/go/keepclient/support.go b/sdk/go/keepclient/support.go
index d3d799dc5d..142f1d2151 100644
--- a/sdk/go/keepclient/support.go
+++ b/sdk/go/keepclient/support.go
@@ -12,30 +12,16 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"log"
 	"math/rand"
 	"net/http"
-	"os"
 	"strconv"
 	"strings"
 	"time"
 
 	"git.arvados.org/arvados.git/sdk/go/arvados"
-	"git.arvados.org/arvados.git/sdk/go/arvadosclient"
 	"git.arvados.org/arvados.git/sdk/go/asyncbuf"
 )
 
-// DebugPrintf emits debug messages. The easiest way to enable
-// keepclient debug messages in your application is to assign
-// log.Printf to DebugPrintf.
-var DebugPrintf = func(string, ...interface{}) {}
-
-func init() {
-	if arvadosclient.StringBool(os.Getenv("ARVADOS_DEBUG")) {
-		DebugPrintf = log.Printf
-	}
-}
-
 type keepService struct {
 	Uuid     string `json:"uuid"`
 	Hostname string `json:"service_host"`
@@ -70,7 +56,7 @@ func (kc *KeepClient) uploadToKeepServer(host string, hash string, classesTodo [
 	var err error
 	var url = fmt.Sprintf("%s/%s", host, hash)
 	if req, err = http.NewRequest("PUT", url, nil); err != nil {
-		DebugPrintf("DEBUG: [%s] Error creating request PUT %v error: %v", reqid, url, err.Error())
+		kc.debugf("[%s] Error creating request: PUT %s error: %s", reqid, url, err)
 		uploadStatusChan <- uploadStatus{err, url, 0, 0, nil, ""}
 		return
 	}
@@ -94,7 +80,7 @@ func (kc *KeepClient) uploadToKeepServer(host string, hash string, classesTodo [
 
 	var resp *http.Response
 	if resp, err = kc.httpClient().Do(req); err != nil {
-		DebugPrintf("DEBUG: [%s] Upload failed %v error: %v", reqid, url, err.Error())
+		kc.debugf("[%s] Upload failed: %s error: %s", reqid, url, err)
 		uploadStatusChan <- uploadStatus{err, url, 0, 0, nil, err.Error()}
 		return
 	}
@@ -106,7 +92,7 @@ func (kc *KeepClient) uploadToKeepServer(host string, hash string, classesTodo [
 	scc := resp.Header.Get(XKeepStorageClassesConfirmed)
 	classesStored, err := parseStorageClassesConfirmedHeader(scc)
 	if err != nil {
-		DebugPrintf("DEBUG: [%s] Ignoring invalid %s header %q: %s", reqid, XKeepStorageClassesConfirmed, scc, err)
+		kc.debugf("[%s] Ignoring invalid %s header %q: %s", reqid, XKeepStorageClassesConfirmed, scc, err)
 	}
 
 	defer resp.Body.Close()
@@ -115,16 +101,16 @@ func (kc *KeepClient) uploadToKeepServer(host string, hash string, classesTodo [
 	respbody, err2 := ioutil.ReadAll(&io.LimitedReader{R: resp.Body, N: 4096})
 	response := strings.TrimSpace(string(respbody))
 	if err2 != nil && err2 != io.EOF {
-		DebugPrintf("DEBUG: [%s] Upload %v error: %v response: %v", reqid, url, err2.Error(), response)
+		kc.debugf("[%s] Upload %s error: %s response: %s", reqid, url, err2, response)
 		uploadStatusChan <- uploadStatus{err2, url, resp.StatusCode, rep, classesStored, response}
 	} else if resp.StatusCode == http.StatusOK {
-		DebugPrintf("DEBUG: [%s] Upload %v success", reqid, url)
+		kc.debugf("[%s] Upload %s success", reqid, url)
 		uploadStatusChan <- uploadStatus{nil, url, resp.StatusCode, rep, classesStored, response}
 	} else {
 		if resp.StatusCode >= 300 && response == "" {
 			response = resp.Status
 		}
-		DebugPrintf("DEBUG: [%s] Upload %v error: %v response: %v", reqid, url, resp.StatusCode, response)
+		kc.debugf("[%s] Upload %s status: %d %s", reqid, url, resp.StatusCode, response)
 		uploadStatusChan <- uploadStatus{errors.New(resp.Status), url, resp.StatusCode, rep, classesStored, response}
 	}
 }
@@ -255,7 +241,7 @@ func (kc *KeepClient) httpBlockWrite(ctx context.Context, req arvados.BlockWrite
 			for active*replicasPerThread < maxConcurrency {
 				// Start some upload requests
 				if nextServer < len(sv) {
-					DebugPrintf("DEBUG: [%s] Begin upload %s to %s", req.RequestID, req.Hash, sv[nextServer])
+					kc.debugf("[%s] Begin upload %s to %s", req.RequestID, req.Hash, sv[nextServer])
 					go kc.uploadToKeepServer(sv[nextServer], req.Hash, classesTodo, getReader(), uploadStatusChan, req.DataSize, req.RequestID)
 					nextServer++
 					active++
@@ -272,7 +258,7 @@ func (kc *KeepClient) httpBlockWrite(ctx context.Context, req arvados.BlockWrite
 				}
 			}
 
-			DebugPrintf("DEBUG: [%s] Replicas remaining to write: %v active uploads: %v", req.RequestID, replicasTodo, active)
+			kc.debugf("[%s] Replicas remaining to write: %d active uploads: %d", req.RequestID, replicasTodo, active)
 			if active < 1 {
 				break
 			}
diff --git a/services/keep-web/cache.go b/services/keep-web/cache.go
index 3ccedf3a07..b5b6cc4fa5 100644
--- a/services/keep-web/cache.go
+++ b/services/keep-web/cache.go
@@ -179,6 +179,7 @@ func (c *cache) checkout(token string) (*cachedSession, error) {
 		}
 		client.AuthToken = token
 		client.Timeout = time.Minute
+		client.Logger = c.logger
 		// A non-empty origin header tells controller to
 		// prioritize our traffic as interactive, which is
 		// true most of the time.

commit 7b2e98f3880b8c3dfc1155a29dd3a9b36f5d7976
Author: Tom Clegg <tom at curii.com>
Date:   Fri May 10 14:34:57 2024 -0400

    21766: Fix WebDAVCache.DiskCacheSize config being ignored.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keep-web/cache.go b/services/keep-web/cache.go
index d443bc0829..3ccedf3a07 100644
--- a/services/keep-web/cache.go
+++ b/services/keep-web/cache.go
@@ -188,11 +188,13 @@ func (c *cache) checkout(token string) (*cachedSession, error) {
 		if err != nil {
 			return nil, err
 		}
+		kc := keepclient.New(arvadosclient)
+		kc.DiskCacheSize = c.cluster.Collections.WebDAVCache.DiskCacheSize
 		sess = &cachedSession{
 			cache:         c,
 			client:        client,
 			arvadosclient: arvadosclient,
-			keepclient:    keepclient.New(arvadosclient),
+			keepclient:    kc,
 		}
 		c.sessions[token] = sess
 	}

commit f15670e518fb8bf8b2fd26eb3efd6d34c2f8b78b
Author: Tom Clegg <tom at curii.com>
Date:   Thu May 9 17:29:39 2024 -0400

    21766: Fix disk cache size percentage calculation.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index b045553b23..3fef27a736 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -751,8 +751,8 @@ Clusters:
         TTL: 300s
 
         # Maximum amount of data cached in /var/cache/arvados/keep.
-        # Can be given as a percentage ("10%") or a number of bytes
-        # ("10 GiB")
+        # Can be given as a percentage of filesystem size ("10%") or a
+        # number of bytes ("10 GiB")
         DiskCacheSize: 10%
 
         # Approximate memory limit (in bytes) for session cache.
diff --git a/sdk/go/arvados/keep_cache.go b/sdk/go/arvados/keep_cache.go
index 108081d5ac..72a0031cac 100644
--- a/sdk/go/arvados/keep_cache.go
+++ b/sdk/go/arvados/keep_cache.go
@@ -113,7 +113,10 @@ func (cache *DiskCache) setup() {
 	defer sharedCachesLock.Unlock()
 	dir := cache.Dir
 	if sharedCaches[dir] == nil {
+		cache.debugf("initializing sharedCache using %s with max size %d", dir, cache.MaxSize)
 		sharedCaches[dir] = &sharedCache{dir: dir, maxSize: cache.MaxSize}
+	} else {
+		cache.debugf("using existing sharedCache using %s with max size %d (would have initialized with %d)", dir, sharedCaches[dir].maxSize, cache.MaxSize)
 	}
 	cache.sharedCache = sharedCaches[dir]
 }
@@ -623,8 +626,9 @@ func (cache *DiskCache) tidy() {
 			}
 			var stat unix.Statfs_t
 			if nil == unix.Statfs(cache.dir, &stat) {
-				maxsize = int64(stat.Bavail) * stat.Bsize * pct / 100
+				maxsize = int64(stat.Blocks) * stat.Bsize * pct / 100
 				atomic.StoreInt64(&cache.defaultMaxSize, maxsize)
+				cache.debugf("setting cache size %d = blocks %d * bsize %d * pct %d / 100", maxsize, stat.Blocks, stat.Bsize, pct)
 			} else {
 				// In this case we will set
 				// defaultMaxSize below after

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list