5538: add test with a connection idle for longer than MaxIdleConnectionDuration

diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index 1bc6f80f2cae1a2e17db60f2d5f844f542233f1e..18e1074bf6f6c801c21593bc57586a902807f8b4 100644 (file)
--- a/sdk/go/arvadosclient/arvadosclient.go
+++ b/sdk/go/arvadosclient/arvadosclient.go
@@ -26,7 +26,10 @@ var MissingArvadosApiHost = errors.New("Missing required environment variable AR
 var MissingArvadosApiToken = errors.New("Missing required environment variable ARVADOS_API_TOKEN")
 var ErrInvalidArgument = errors.New("Invalid argument")
 
-// Before a POST or DELERE request, close any connections that were idle for this long
+// A common failure mode is to reuse a keepalive connection that has been
+// terminated (in a way that we can't detect) for being idle too long.
+// POST and DELETE are not safe to retry automatically, so we minimize
+// such failures by always using a new or recently active socket.
 var MaxIdleConnectionDuration = 30 * time.Second
 
 // Indicates an error that was returned by the API server.
@@ -166,7 +169,8 @@ func (c ArvadosClient) CallRaw(method string, resourceType string, uuid string,
                req.Header.Add("X-External-Client", "1")
        }
 
-       // Before a POST or DELETE, close any idle connections
+       // POST and DELETE are not safe to retry automatically, so we minimize
+       // such failures by always using a new or recently active socket
        if method == "POST" || method == "DELETE" {
                if time.Since(c.lastClosedIdlesAt) > MaxIdleConnectionDuration {
                        c.lastClosedIdlesAt = time.Now()

diff --git a/sdk/go/arvadosclient/arvadosclient_test.go b/sdk/go/arvadosclient/arvadosclient_test.go
index 2c508dcb4a1100cf4609fc3ff3387ac089c6ab26..36b3165e9acee4dbf5a93d6b3acd377a54cbcf9c 100644 (file)
--- a/sdk/go/arvadosclient/arvadosclient_test.go
+++ b/sdk/go/arvadosclient/arvadosclient_test.go
@@ -6,6 +6,7 @@ import (
        "net/http"
        "os"
        "testing"
+       "time"
 )
 
 // Gocheck boilerplate
@@ -100,41 +101,50 @@ func (s *ServerRequiredSuite) TestInvalidResourceType(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestCreatePipelineTemplate(c *C) {
-       arv, err := MakeArvadosClient()
-
-       getback := make(Dict)
-       err = arv.Create("pipeline_templates",
-               Dict{"pipeline_template": Dict{
-                       "name": "tmp",
-                       "components": Dict{
-                               "c1": map[string]string{"script": "script1"},
-                               "c2": map[string]string{"script": "script2"}}}},
-               &getback)
-       c.Assert(err, Equals, nil)
-       c.Assert(getback["name"], Equals, "tmp")
-       c.Assert(getback["components"].(map[string]interface{})["c2"].(map[string]interface{})["script"], Equals, "script2")
-
-       uuid := getback["uuid"].(string)
-
-       getback = make(Dict)
-       err = arv.Get("pipeline_templates", uuid, nil, &getback)
-       c.Assert(err, Equals, nil)
-       c.Assert(getback["name"], Equals, "tmp")
-       c.Assert(getback["components"].(map[string]interface{})["c1"].(map[string]interface{})["script"], Equals, "script1")
-
-       getback = make(Dict)
-       err = arv.Update("pipeline_templates", uuid,
-               Dict{
-                       "pipeline_template": Dict{"name": "tmp2"}},
-               &getback)
-       c.Assert(err, Equals, nil)
-       c.Assert(getback["name"], Equals, "tmp2")
-
-       c.Assert(getback["uuid"].(string), Equals, uuid)
-       getback = make(Dict)
-       err = arv.Delete("pipeline_templates", uuid, nil, &getback)
-       c.Assert(err, Equals, nil)
-       c.Assert(getback["name"], Equals, "tmp2")
+       for _, idleConnections := range []bool{
+               false,
+               true,
+       } {
+               arv, err := MakeArvadosClient()
+
+               if idleConnections {
+                       arv.lastClosedIdlesAt = time.Now().Add(-time.Minute)
+               }
+
+               getback := make(Dict)
+               err = arv.Create("pipeline_templates",
+                       Dict{"pipeline_template": Dict{
+                               "name": "tmp",
+                               "components": Dict{
+                                       "c1": map[string]string{"script": "script1"},
+                                       "c2": map[string]string{"script": "script2"}}}},
+                       &getback)
+               c.Assert(err, Equals, nil)
+               c.Assert(getback["name"], Equals, "tmp")
+               c.Assert(getback["components"].(map[string]interface{})["c2"].(map[string]interface{})["script"], Equals, "script2")
+
+               uuid := getback["uuid"].(string)
+
+               getback = make(Dict)
+               err = arv.Get("pipeline_templates", uuid, nil, &getback)
+               c.Assert(err, Equals, nil)
+               c.Assert(getback["name"], Equals, "tmp")
+               c.Assert(getback["components"].(map[string]interface{})["c1"].(map[string]interface{})["script"], Equals, "script1")
+
+               getback = make(Dict)
+               err = arv.Update("pipeline_templates", uuid,
+                       Dict{
+                               "pipeline_template": Dict{"name": "tmp2"}},
+                       &getback)
+               c.Assert(err, Equals, nil)
+               c.Assert(getback["name"], Equals, "tmp2")
+
+               c.Assert(getback["uuid"].(string), Equals, uuid)
+               getback = make(Dict)
+               err = arv.Delete("pipeline_templates", uuid, nil, &getback)
+               c.Assert(err, Equals, nil)
+               c.Assert(getback["name"], Equals, "tmp2")
+       }
 }
 
 func (s *ServerRequiredSuite) TestErrorResponse(c *C) {