Merge pull request #1269 from dabooz/issue1266

Issue 1266 - large model object upload times out

cli/cliutils/cliutils.go

@@ -1019,6 +1019,16 @@ func invokeRestApi(httpClient *http.Client, method string, url string, credentia
 		// requestBody is nil if body is nil.
 		requestBody, bodyLen, bodyType := createRequestBody(body, apiMsg)
 
+		// If we're retrying with an os.File body, then re-open it.
+		if retryCount > 1 {
+			file := body.(*os.File)
+			if rb, err := os.Open(file.Name()); err != nil {
+				Fatal(CLI_INPUT_ERROR, msgPrinter.Sprintf("unable to open object file %v: %v", file.Name(), err))
+			} else {
+				requestBody = rb
+			}
+		}
+
 		// Create the request and run it
 		req, err := http.NewRequest(method, url, requestBody)
 		if err != nil {
@@ -1401,24 +1411,39 @@ func GetHTTPClient(timeout int) *http.Client {
 		skipSSL = true
 	}
 
-	responseTimeout := 20
-	if timeout == 0 {
-		responseTimeout = 0
+	// Set request timeout based on environment variables and input values. The environment variable always overrides the
+	// input parameter. The other timeouts are subject to the timeout setting also.
+	requestTimeout := timeout
+
+	if envTimeout := os.Getenv(config.HTTPRequestTimeoutOverride); envTimeout != "" {
+		if t, err := strconv.Atoi(envTimeout); err == nil {
+			requestTimeout = t
+		} else {
+			Warning(i18n.GetMessagePrinter().Sprintf("Unable to use %v to set the request timeout, the value is not a valid number: %v", config.HTTPRequestTimeoutOverride, envTimeout))
+		}
 	}
 
+	responseTimeout := int(float64(requestTimeout)*0.8)
+	dialTimeout := int(float64(requestTimeout)*0.5)
+	keepAlive := requestTimeout*2
+	TLSHandshake := dialTimeout
+	expectContinue := int(float64(requestTimeout)*0.5)
+
+	Verbose(i18n.GetMessagePrinter().Sprintf("HTTP request timeout set to %v seconds", requestTimeout))
+
 	return &http.Client{
 		// remember that this timeout is for the whole request, including
 		// body reading. This means that you must set the timeout according
 		// to the total payload size you expect
-		Timeout: time.Second * time.Duration(timeout),
+		Timeout: time.Second * time.Duration(requestTimeout),
 		Transport: &http.Transport{
 			Dial: (&net.Dialer{
-				Timeout:   20 * time.Second,
-				KeepAlive: 60 * time.Second,
+				Timeout:   time.Duration(dialTimeout) * time.Second,
+				KeepAlive: time.Duration(keepAlive) * time.Second,
 			}).Dial,
-			TLSHandshakeTimeout:   20 * time.Second,
+			TLSHandshakeTimeout:   time.Duration(TLSHandshake) * time.Second,
 			ResponseHeaderTimeout: time.Duration(responseTimeout) * time.Second,
-			ExpectContinueTimeout: 8 * time.Second,
+			ExpectContinueTimeout: time.Duration(expectContinue) * time.Second,
 			MaxIdleConns:          config.MaxHTTPIdleConnections,
 			IdleConnTimeout:       config.HTTPIdleConnectionTimeoutS * time.Second,
 			TLSClientConfig: &tls.Config{

cli/sync_service/object.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"github.com/open-horizon/anax/cli/cliutils"
+	"github.com/open-horizon/anax/config"
 	"github.com/open-horizon/anax/exchange"
 	"github.com/open-horizon/anax/i18n"
 	"github.com/open-horizon/edge-sync-service/common"
@@ -232,10 +233,22 @@ func ObjectPublish(org string, userPw string, objType string, objId string, objP
 		}
 		defer file.Close()
 
+		// Establish the HTTP request override because the upload could take some time.
+		setHTTPOverride := false
+		if os.Getenv(config.HTTPRequestTimeoutOverride) == "" {
+			setHTTPOverride = true
+			os.Setenv(config.HTTPRequestTimeoutOverride, "0")
+		}
+
 		// Stream the file to the MMS (CSS).
 		urlPath = path.Join("api/v1/objects/", org, objectMeta.ObjectType, objectMeta.ObjectID, "data")
 		cliutils.ExchangePutPost("Model Management Service", http.MethodPut, cliutils.GetMMSUrl(), urlPath, cliutils.OrgAndCreds(org, userPw), []int{204}, file)
 
+		// Restore HTTP request override if necessary.
+		if setHTTPOverride {
+			os.Setenv(config.HTTPRequestTimeoutOverride, "")
+		}
+
 		cliutils.Verbose(msgPrinter.Sprintf("Object %v uploaded to org %v in the Model Management Service", objFile, org))
 	}
 

config/constants.go

@@ -11,9 +11,12 @@ const MICROSERVICE_EXEC_TIMEOUT = 180
 // MaxHTTPIdleConnections see https://golang.org/pkg/net/http/
 const MaxHTTPIdleConnections = 20
 
-// HTTPIdleConnectionTimeoutS see https://golang.org/pkg/net/http/
+// HTTPRequestTimeoutS see https://golang.org/pkg/net/http/
 const HTTPRequestTimeoutS = 30
 
+// HTTPRequestTimeoutOverride environment variable
+const HTTPRequestTimeoutOverride = "HZN_HTTP_TIMEOUT"
+
 // HTTPIdleConnectionTimeoutS see https://golang.org/pkg/net/http/
 const HTTPIdleConnectionTimeoutS = 120
 

test/gov/sync_service_test.sh

@@ -44,4 +44,49 @@ then
   exit -1
 fi
 
+#Test timeout support for upload of large files to the CSS
+dd if=/dev/zero of=/tmp/data.txt count=512 bs=1048576
+
+RESOURCE_ORG1=e2edev@somecomp.com
+RESOURCE_TYPE=test
+
+export HZN_FSS_CSSURL=${CSS_URL}
+
+read -d '' resmeta <<EOF
+{
+  "objectID": "test1",
+  "objectType": "test",
+  "destinationType": "test",
+  "version": "1.0.0",
+  "description": "a file"
+}
+EOF
+
+echo "$resmeta" > /tmp/meta.json
+
+hzn mms object publish -m /tmp/meta.json -f /tmp/data.txt
+RC=$?
+if [ $RC -ne 0 ]
+then
+  echo -e "Got unexpected error uploading 512MB model object: $RC"
+  exit -1
+fi
+
+# Now, shorten the HTTP request timeout so that the upload fails. Internally, the CLI will retry
+# before giving up with the appropriate HTTP Client timeout error.
+export HZN_HTTP_TIMEOUT="2"
+
+hzn mms object publish -m /tmp/meta.json -f /tmp/data.txt
+RC=$?
+if [ $RC -eq 5 ]
+then
+  echo -e "Got expected error with 521MB object upload using short HTTP request timeout: $RC"
+else
+  echo -e "Got unexpected error with 521MB object upload using short HTTP request timeout: $RC"
+  exit -1
+fi
+
+# Reset the HTTP timeout env var to the default for the CLI.
+unset HZN_HTTP_TIMEOUT
+
 echo "Testing model management APIs successful"