[arvados] created: 2.7.0-5501-g504d5fea43
git repository hosting
git at public.arvados.org
Fri Dec 1 20:49:06 UTC 2023
at 504d5fea43c831adb924da5df84825c4e2fabb45 (commit)
commit 504d5fea43c831adb924da5df84825c4e2fabb45
Author: Tom Clegg <tom at curii.com>
Date: Fri Dec 1 15:48:22 2023 -0500
21227: Fix captured loop variable.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/controller/localdb/container_test.go b/lib/controller/localdb/container_test.go
index 946eb32b9d..86ae714ba9 100644
--- a/lib/controller/localdb/container_test.go
+++ b/lib/controller/localdb/container_test.go
@@ -204,6 +204,7 @@ func (s *containerSuite) TestUpdatePriorityMultiLevelWorkflow(c *C) {
defer deadlockCancel()
for _, cr := range allcrs {
if strings.Contains(cr.Command[2], " j ") && !strings.Contains(cr.Command[2], " k ") {
+ cr := cr
wg.Add(1)
go func() {
defer wg.Done()
commit 8989c3ca9c63dc05975b2c02e56c2031246beb96
Author: Tom Clegg <tom at curii.com>
Date: Fri Dec 1 15:48:13 2023 -0500
21227: Fix copied mutexes.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/boot/supervisor.go b/lib/boot/supervisor.go
index e25fb8cdba..ac269b933a 100644
--- a/lib/boot/supervisor.go
+++ b/lib/boot/supervisor.go
@@ -204,15 +204,24 @@ func (super *Supervisor) Wait() error {
func (super *Supervisor) startFederation(cfg *arvados.Config) {
super.children = map[string]*Supervisor{}
for id, cc := range cfg.Clusters {
- super2 := *super
yaml, err := json.Marshal(arvados.Config{Clusters: map[string]arvados.Cluster{id: cc}})
if err != nil {
panic(fmt.Sprintf("json.Marshal partial config: %s", err))
}
- super2.ConfigYAML = string(yaml)
- super2.ConfigPath = "-"
- super2.children = nil
-
+ super2 := &Supervisor{
+ ConfigPath: "-",
+ ConfigYAML: string(yaml),
+ SourcePath: super.SourcePath,
+ SourceVersion: super.SourceVersion,
+ ClusterType: super.ClusterType,
+ ListenHost: super.ListenHost,
+ ControllerAddr: super.ControllerAddr,
+ NoWorkbench1: super.NoWorkbench1,
+ NoWorkbench2: super.NoWorkbench2,
+ OwnTemporaryDatabase: super.OwnTemporaryDatabase,
+ Stdin: super.Stdin,
+ Stderr: super.Stderr,
+ }
if super2.ClusterType == "test" {
super2.Stderr = &service.LogPrefixer{
Writer: super.Stderr,
@@ -220,7 +229,7 @@ func (super *Supervisor) startFederation(cfg *arvados.Config) {
}
}
super2.Start(super.ctx)
- super.children[id] = &super2
+ super.children[id] = super2
}
}
diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 68ac886ddd..86001c01e0 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -120,6 +120,27 @@ type KeepClient struct {
disableDiscovery bool
}
+func (kc *KeepClient) Clone() *KeepClient {
+ kc.lock.Lock()
+ defer kc.lock.Unlock()
+ return &KeepClient{
+ Arvados: kc.Arvados,
+ Want_replicas: kc.Want_replicas,
+ localRoots: kc.localRoots,
+ writableLocalRoots: kc.writableLocalRoots,
+ gatewayRoots: kc.gatewayRoots,
+ HTTPClient: kc.HTTPClient,
+ Retries: kc.Retries,
+ BlockCache: kc.BlockCache,
+ RequestID: kc.RequestID,
+ StorageClasses: kc.StorageClasses,
+ DefaultStorageClasses: kc.DefaultStorageClasses,
+ replicasPerService: kc.replicasPerService,
+ foundNonDiskSvc: kc.foundNonDiskSvc,
+ disableDiscovery: kc.disableDiscovery,
+ }
+}
+
func (kc *KeepClient) loadDefaultClasses() error {
scData, err := kc.Arvados.ClusterConfig("StorageClasses")
if err != nil {
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 2090c50686..a79883147b 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -175,13 +175,18 @@ func (h *proxyHandler) checkAuthorizationHeader(req *http.Request) (pass bool, t
return true, tok, user
}
-// We need to make a private copy of the default http transport early
-// in initialization, then make copies of our private copy later. It
-// won't be safe to copy http.DefaultTransport itself later, because
-// its private mutexes might have already been used. (Without this,
-// the test suite sometimes panics "concurrent map writes" in
-// net/http.(*Transport).removeIdleConnLocked().)
-var defaultTransport = *(http.DefaultTransport.(*http.Transport))
+// We can't copy the default http transport because http.Transport has
+// a mutex field, so we make our own using the values of the exported
+// fields.
+var defaultTransport = http.Transport{
+ Proxy: http.DefaultTransport.(*http.Transport).Proxy,
+ DialContext: http.DefaultTransport.(*http.Transport).DialContext,
+ ForceAttemptHTTP2: http.DefaultTransport.(*http.Transport).ForceAttemptHTTP2,
+ MaxIdleConns: http.DefaultTransport.(*http.Transport).MaxIdleConns,
+ IdleConnTimeout: http.DefaultTransport.(*http.Transport).IdleConnTimeout,
+ TLSHandshakeTimeout: http.DefaultTransport.(*http.Transport).TLSHandshakeTimeout,
+ ExpectContinueTimeout: http.DefaultTransport.(*http.Transport).ExpectContinueTimeout,
+}
type proxyHandler struct {
http.Handler
@@ -195,14 +200,23 @@ type proxyHandler struct {
func newHandler(ctx context.Context, kc *keepclient.KeepClient, timeout time.Duration, cluster *arvados.Cluster) (service.Handler, error) {
rest := mux.NewRouter()
- transport := defaultTransport
- transport.DialContext = (&net.Dialer{
- Timeout: keepclient.DefaultConnectTimeout,
- KeepAlive: keepclient.DefaultKeepAlive,
- DualStack: true,
- }).DialContext
- transport.TLSClientConfig = arvadosclient.MakeTLSConfig(kc.Arvados.ApiInsecure)
- transport.TLSHandshakeTimeout = keepclient.DefaultTLSHandshakeTimeout
+ // We can't copy the default http transport because
+ // http.Transport has a mutex field, so we copy the fields
+ // that we know have non-zero values in http.DefaultTransport.
+ transport := &http.Transport{
+ Proxy: http.DefaultTransport.(*http.Transport).Proxy,
+ ForceAttemptHTTP2: http.DefaultTransport.(*http.Transport).ForceAttemptHTTP2,
+ MaxIdleConns: http.DefaultTransport.(*http.Transport).MaxIdleConns,
+ IdleConnTimeout: http.DefaultTransport.(*http.Transport).IdleConnTimeout,
+ ExpectContinueTimeout: http.DefaultTransport.(*http.Transport).ExpectContinueTimeout,
+ DialContext: (&net.Dialer{
+ Timeout: keepclient.DefaultConnectTimeout,
+ KeepAlive: keepclient.DefaultKeepAlive,
+ DualStack: true,
+ }).DialContext,
+ TLSClientConfig: arvadosclient.MakeTLSConfig(kc.Arvados.ApiInsecure),
+ TLSHandshakeTimeout: keepclient.DefaultTLSHandshakeTimeout,
+ }
cacheQ, err := lru.New2Q(500)
if err != nil {
@@ -213,7 +227,7 @@ func newHandler(ctx context.Context, kc *keepclient.KeepClient, timeout time.Dur
Handler: rest,
KeepClient: kc,
timeout: timeout,
- transport: &transport,
+ transport: transport,
apiTokenCache: &apiTokenCache{
tokens: cacheQ,
expireTime: 300,
@@ -566,7 +580,7 @@ func (h *proxyHandler) Index(resp http.ResponseWriter, req *http.Request) {
}
func (h *proxyHandler) makeKeepClient(req *http.Request) *keepclient.KeepClient {
- kc := *h.KeepClient
+ kc := h.KeepClient.Clone()
kc.RequestID = req.Header.Get("X-Request-Id")
kc.HTTPClient = &proxyClient{
client: &http.Client{
@@ -575,5 +589,5 @@ func (h *proxyHandler) makeKeepClient(req *http.Request) *keepclient.KeepClient
},
proto: req.Proto,
}
- return &kc
+ return kc
}
diff --git a/services/keepstore/proxy_remote.go b/services/keepstore/proxy_remote.go
index 526bc25299..66a7b43751 100644
--- a/services/keepstore/proxy_remote.go
+++ b/services/keepstore/proxy_remote.go
@@ -130,14 +130,14 @@ func (rp *remoteProxy) remoteClient(remoteID string, remoteCluster arvados.Remot
}
accopy := *kc.Arvados
accopy.ApiToken = token
- kccopy := *kc
+ kccopy := kc.Clone()
kccopy.Arvados = &accopy
token, err := auth.SaltToken(token, remoteID)
if err != nil {
return nil, err
}
kccopy.Arvados.ApiToken = token
- return &kccopy, nil
+ return kccopy, nil
}
var localOrRemoteSignature = regexp.MustCompile(`\+[AR][^\+]*`)
diff --git a/services/keepstore/pull_worker.go b/services/keepstore/pull_worker.go
index abe3dc3857..b9194fe6f6 100644
--- a/services/keepstore/pull_worker.go
+++ b/services/keepstore/pull_worker.go
@@ -50,7 +50,7 @@ func (h *handler) pullItemAndProcess(pullRequest PullRequest) error {
// Make a private copy of keepClient so we can set
// ServiceRoots to the source servers specified in the pull
// request.
- keepClient := *h.keepClient
+ keepClient := h.keepClient.Clone()
serviceRoots := make(map[string]string)
for _, addr := range pullRequest.Servers {
serviceRoots[addr] = addr
@@ -59,7 +59,7 @@ func (h *handler) pullItemAndProcess(pullRequest PullRequest) error {
signedLocator := SignLocator(h.Cluster, pullRequest.Locator, keepClient.Arvados.ApiToken, time.Now().Add(time.Minute))
- reader, contentLen, _, err := GetContent(signedLocator, &keepClient)
+ reader, contentLen, _, err := GetContent(signedLocator, keepClient)
if err != nil {
return err
}
commit 3e65a597ea81688774098a921246006ec3e933f6
Author: Tom Clegg <tom at curii.com>
Date: Fri Dec 1 15:47:06 2023 -0500
21227: Fix unbuffered channels used for signal.Notify.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/boot/supervisor.go b/lib/boot/supervisor.go
index 28be8f3a14..e25fb8cdba 100644
--- a/lib/boot/supervisor.go
+++ b/lib/boot/supervisor.go
@@ -111,7 +111,7 @@ func (super *Supervisor) Start(ctx context.Context) {
super.ctx, super.cancel = context.WithCancel(ctx)
super.done = make(chan struct{})
- sigch := make(chan os.Signal)
+ sigch := make(chan os.Signal, 1)
signal.Notify(sigch, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP)
go func() {
defer signal.Stop(sigch)
diff --git a/services/keep-balance/server.go b/services/keep-balance/server.go
index b20144e3af..480791ffa2 100644
--- a/services/keep-balance/server.go
+++ b/services/keep-balance/server.go
@@ -98,9 +98,7 @@ func (srv *Server) runForever(ctx context.Context) error {
ticker := time.NewTicker(time.Duration(srv.Cluster.Collections.BalancePeriod))
- // The unbuffered channel here means we only hear SIGUSR1 if
- // it arrives while we're waiting in select{}.
- sigUSR1 := make(chan os.Signal)
+ sigUSR1 := make(chan os.Signal, 1)
signal.Notify(sigUSR1, syscall.SIGUSR1)
logger.Info("acquiring service lock")
commit d7b8f2a876c797c22bcb8594f73624402d758e18
Author: Tom Clegg <tom at curii.com>
Date: Fri Dec 1 15:44:31 2023 -0500
21227: Fail tests on 'go vet' problems.
...except "literal uses unkeyed fields", of which there are currently
many instances, mostly in tests.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/build/run-tests.sh b/build/run-tests.sh
index 83688e8727..70437bd21c 100755
--- a/build/run-tests.sh
+++ b/build/run-tests.sh
@@ -1000,6 +1000,7 @@ test_gofmt() {
cd "$WORKSPACE" || return 1
dirs=$(ls -d */ | egrep -v 'vendor|tmp')
[[ -z "$(gofmt -e -d $dirs | tee -a /dev/stderr)" ]]
+ go vet -composites=false ./...
}
test_services/api() {
commit 79036634292cc4ed13db98a834f33d617c85b1cd
Author: Tom Clegg <tom at curii.com>
Date: Fri Dec 1 15:43:56 2023 -0500
21227: Fix copied mutex after first use.
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 735a44d24c..172763fe80 100644
--- a/sdk/go/arvados/client.go
+++ b/sdk/go/arvados/client.go
@@ -88,7 +88,10 @@ type Client struct {
// differs from an outgoing connection limit (a feature
// provided by http.Transport) when concurrent calls are
// multiplexed on a single http2 connection.
- requestLimiter requestLimiter
+ //
+ // getRequestLimiter() should always be used, because this can
+ // be nil.
+ requestLimiter *requestLimiter
last503 atomic.Value
}
@@ -150,7 +153,7 @@ func NewClientFromConfig(cluster *Cluster) (*Client, error) {
APIHost: ctrlURL.Host,
Insecure: cluster.TLS.Insecure,
Timeout: 5 * time.Minute,
- requestLimiter: requestLimiter{maxlimit: int64(cluster.API.MaxConcurrentRequests / 4)},
+ requestLimiter: &requestLimiter{maxlimit: int64(cluster.API.MaxConcurrentRequests / 4)},
}, nil
}
@@ -287,7 +290,7 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
rclient.RetryMax = 0
}
rclient.CheckRetry = func(ctx context.Context, resp *http.Response, respErr error) (bool, error) {
- if c.requestLimiter.Report(resp, respErr) {
+ if c.getRequestLimiter().Report(resp, respErr) {
c.last503.Store(time.Now())
}
if c.Timeout == 0 {
@@ -314,9 +317,10 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
}
rclient.Logger = nil
- c.requestLimiter.Acquire(ctx)
+ limiter := c.getRequestLimiter()
+ limiter.Acquire(ctx)
if ctx.Err() != nil {
- c.requestLimiter.Release()
+ limiter.Release()
cancel()
return nil, ctx.Err()
}
@@ -328,7 +332,7 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
}
}
if err != nil {
- c.requestLimiter.Release()
+ limiter.Release()
cancel()
return nil, err
}
@@ -338,7 +342,7 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
resp.Body = cancelOnClose{
ReadCloser: resp.Body,
cancel: func() {
- c.requestLimiter.Release()
+ limiter.Release()
cancel()
},
}
@@ -352,6 +356,19 @@ func (c *Client) Last503() time.Time {
return t
}
+// globalRequestLimiter doesn't have a maximum number of outgoing
+// connections, but is just used to backoff after 503 errors.
+var globalRequestLimiter requestLimiter
+
+// Get this client's requestLimiter, or the global requestLimiter
+// singleton if the client doesn't have its own.
+func (c *Client) getRequestLimiter() *requestLimiter {
+ if c.requestLimiter != nil {
+ return c.requestLimiter
+ }
+ return &globalRequestLimiter
+}
+
// cancelOnClose calls a provided CancelFunc when its wrapped
// ReadCloser's Close() method is called.
type cancelOnClose struct {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list