gpcloud: retry ServiceUnavailable and RequestTimeout errors (#3963)

ServiceUnavailable and RequestTimeout are errors needed retry, this
commit treats them as S3ConnectionError.

Other errors returned by S3 than 500, 503 and RequestTimeout are
still considered as S3LogicError.

gpAux/extensions/gpcloud/include/s3exception.h

@@ -27,7 +27,8 @@ class S3Exception {
     }
 };
 
-// HTTP request failed.
+// HTTP request failed
+// InternalError, RequestTimeout or ServiceUnavailable
 class S3ConnectionError : public S3Exception {
    public:
     S3ConnectionError(const string& msg) : message(msg) {
@@ -79,7 +80,7 @@ class S3FailedAfterRetry : public S3Exception {
     string message;
 };
 
-// HTTP request success, but the data received is not completed.
+// HTTP request successes, but the data is not received completely.
 class S3PartialResponseError : public S3Exception {
    public:
     S3PartialResponseError(uint64_t expected, uint64_t received)
@@ -100,7 +101,7 @@ class S3PartialResponseError : public S3Exception {
     uint64_t receivedLength;
 };
 
-// User press control + C or transaction is aborted.
+// User presses Ctrl + C or the transaction is aborted.
 class S3QueryAbort : public S3Exception {
    public:
     S3QueryAbort() : message("Query is aborted") {
@@ -119,7 +120,8 @@ class S3QueryAbort : public S3Exception {
     string message;
 };
 
-// Used for AWS S3 errors (e.g. 403, 404)
+// AWS S3 responds errors
+// AccessDenied, NoSuchBucket or other kinds of InvalidRequest
 class S3LogicError : public S3Exception {
    public:
     S3LogicError(string code, string msg) : message(msg), awscode(code) {

gpAux/extensions/gpcloud/src/s3interface.cpp

@@ -33,9 +33,29 @@ Response S3InterfaceService::getResponseWithRetries(const string &url, HTTPHeade
                                                     uint64_t retries) {
     string message;
     uint64_t retry = retries;
+
     while (retry--) {
         try {
-            return this->restfulService->get(url, headers);
+            Response response = this->restfulService->get(url, headers);
+
+            if (response.getStatus() == RESPONSE_OK) {
+                return response;
+            }
+
+            S3MessageParser s3msg(response);
+            message = s3msg.getMessage();
+            ResponseCode responseCode = response.getResponseCode();
+
+            if ((responseCode == 500) || (responseCode == 503)) {
+                S3_DIE(S3ConnectionError, message);
+            }
+            if (responseCode == 400) {
+                if (s3msg.getCode().compare("RequestTimeout") == 0) {
+                    S3_DIE(S3ConnectionError, message);
+                }
+            }
+
+            return response;
         } catch (S3ConnectionError &e) {
             message = e.getMessage();
             if (S3QueryIsAbortInProgress()) {
@@ -52,9 +72,29 @@ Response S3InterfaceService::putResponseWithRetries(const string &url, HTTPHeade
                                                     S3VectorUInt8 &data, uint64_t retries) {
     string message;
     uint64_t retry = retries;
+
     while (retry--) {
         try {
-            return this->restfulService->put(url, headers, data);
+            Response response = this->restfulService->put(url, headers, data);
+
+            if (response.getStatus() == RESPONSE_OK) {
+                return response;
+            }
+
+            S3MessageParser s3msg(response);
+            message = s3msg.getMessage();
+            ResponseCode responseCode = response.getResponseCode();
+
+            if ((responseCode == 500) || (responseCode == 503)) {
+                S3_DIE(S3ConnectionError, message);
+            }
+            if (responseCode == 400) {
+                if (s3msg.getCode().compare("RequestTimeout") == 0) {
+                    S3_DIE(S3ConnectionError, message);
+                }
+            }
+
+            return response;
         } catch (S3ConnectionError &e) {
             message = e.getMessage();
             if (S3QueryIsAbortInProgress()) {
@@ -72,9 +112,29 @@ Response S3InterfaceService::postResponseWithRetries(const string &url, HTTPHead
                                                      uint64_t retries) {
     string message;
     uint64_t retry = retries;
+
     while (retry--) {
         try {
-            return this->restfulService->post(url, headers, data);
+            Response response = this->restfulService->post(url, headers, data);
+
+            if (response.getStatus() == RESPONSE_OK) {
+                return response;
+            }
+
+            S3MessageParser s3msg(response);
+            message = s3msg.getMessage();
+            ResponseCode responseCode = response.getResponseCode();
+
+            if ((responseCode == 500) || (responseCode == 503)) {
+                S3_DIE(S3ConnectionError, message);
+            }
+            if (responseCode == 400) {
+                if (s3msg.getCode().compare("RequestTimeout") == 0) {
+                    S3_DIE(S3ConnectionError, message);
+                }
+            }
+
+            return response;
         } catch (S3ConnectionError &e) {
             message = e.getMessage();
             if (S3QueryIsAbortInProgress()) {
@@ -95,9 +155,16 @@ ResponseCode S3InterfaceService::headResponseWithRetries(const string &url, HTTP
                                                          uint64_t retries) {
     string message;
     uint64_t retry = retries;
+
     while (retry--) {
         try {
-            return this->restfulService->head(url, headers);
+            ResponseCode responseCode = this->restfulService->head(url, headers);
+
+            if ((responseCode == 500) || (responseCode == 503)) {
+                S3_DIE(S3ConnectionError, "Server temporary unavailable");
+            }
+
+            return responseCode;
         } catch (S3ConnectionError &e) {
             message = e.getMessage();
             if (S3QueryIsAbortInProgress()) {
@@ -110,13 +177,34 @@ ResponseCode S3InterfaceService::headResponseWithRetries(const string &url, HTTP
 
     S3_DIE(S3FailedAfterRetry, url, retries, message);
 }
+
 Response S3InterfaceService::deleteRequestWithRetries(const string &url, HTTPHeaders &headers,
                                                       uint64_t retries) {
     string message;
     uint64_t retry = retries;
+
     while (retry--) {
         try {
-            return this->restfulService->deleteRequest(url, headers);
+            Response response = this->restfulService->deleteRequest(url, headers);
+
+            if (response.getStatus() == RESPONSE_OK) {
+                return response;
+            }
+
+            S3MessageParser s3msg(response);
+            message = s3msg.getMessage();
+            ResponseCode responseCode = response.getResponseCode();
+
+            if ((responseCode == 500) || (responseCode == 503)) {
+                S3_DIE(S3ConnectionError, message);
+            }
+            if (responseCode == 400) {
+                if (s3msg.getCode().compare("RequestTimeout") == 0) {
+                    S3_DIE(S3ConnectionError, message);
+                }
+            }
+
+            return response;
         } catch (S3ConnectionError &e) {
             message = e.getMessage();
             if (S3QueryIsAbortInProgress()) {
@@ -132,7 +220,7 @@ Response S3InterfaceService::deleteRequestWithRetries(const string &url, HTTPHea
 xmlParserCtxtPtr S3InterfaceService::getXMLContext(Response &response) {
     xmlParserCtxtPtr xmlptr =
         xmlCreatePushParserCtxt(NULL, NULL, (const char *)(response.getRawData().data()),
-                                response.getRawData().size(), "resp.xml");
+                                response.getRawData().size(), "getXMLContext.xml");
     if (xmlptr != NULL) {
         xmlParseChunk(xmlptr, "", 0, 1);
     } else {
@@ -550,17 +638,15 @@ bool S3InterfaceService::abortUpload(const S3Url &s3Url, const string &uploadId)
     }
 }
 
-S3MessageParser::S3MessageParser(const Response &resp) : xmlptr(NULL) {
+S3MessageParser::S3MessageParser(const Response &resp)
+    : xmlptr(NULL), message("Unkown error"), code("Unknown error code") {
     // Compatible S3 services don't always return XML
     if (resp.getRawData().data() == NULL) {
-        message = "Unknown error";
-        code = "Unknown error code";
-
         return;
     }
 
     xmlptr = xmlCreatePushParserCtxt(NULL, NULL, (const char *)(resp.getRawData().data()),
-                                     resp.getRawData().size(), "resp.xml");
+                                     resp.getRawData().size(), "S3MessageParser.xml");
     if (xmlptr != NULL) {
         xmlParseChunk(xmlptr, "", 0, 1);
         message = parseS3Tag("Message");
@@ -576,7 +662,7 @@ S3MessageParser::~S3MessageParser() {
 }
 
 string S3MessageParser::parseS3Tag(const string &tag) {
-    string contentStr;
+    string contentStr("Unknown value");
 
     xmlNode *rootElement = xmlDocGetRootElement(xmlptr->myDoc);
     if (rootElement == NULL) {

gpAux/extensions/gpcloud/src/s3restful_service.cpp

@@ -134,10 +134,6 @@ void S3RESTfulService::performCurl(CURL *curl, Response &response) {
         // Get the HTTP response status code from HTTP header
         curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &responseCode);
 
-        if (responseCode == 500) {
-            S3_DIE(S3ConnectionError, "Server temporary unavailable");
-        }
-
         response.FillResponse(responseCode);
     }
 }

gpAux/extensions/gpcloud/test/s3restful_service_test.cpp

@@ -337,4 +337,4 @@ TEST(S3RESTfulService, GetWithWrongProxyUrl) {
     string url = "https://www.bing.com/";
 
     EXPECT_THROW(service.get(url, headers), S3ResolveError);
-}
\ No newline at end of file
+}