Merge pull request #1651 from satherton/reduce-http-error-noise

For HTTP requests, a missing request ID in the response is ignored if…
apple · Jun 4, 2019 · a6c0964 · a6c0964
2 parents 50f963d + 08e7f41
commit a6c0964
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 9 deletions.
diff --git a/documentation/sphinx/source/release-notes.rst b/documentation/sphinx/source/release-notes.rst
@@ -9,6 +9,7 @@ Fixes
 -----
 
 * Sometimes a minority of coordinators would not converge to the leader. `(PR #1649) <https://github.com/apple/foundationdb/pull/1649>`_
+* HTTP responses indicating a server-side error are no longer expected to contain a ResponseID header. `(PR #1651) <https://github.com/apple/foundationdb/pull/1651>`_
 
 6.1.8
 =====

diff --git a/fdbclient/HTTP.actor.cpp b/fdbclient/HTTP.actor.cpp
@@ -392,17 +392,17 @@ namespace HTTP {
 					responseID = iid->second;
 				}
 				event.detail("RequestIDReceived", responseID);
-				if(requestID != responseID) {
-					err = http_bad_request_id();
 
-					// Log a non-debug a error
-					Severity sev = SevError;
-					// If the response code is 5xx (server error) and the responseID is empty then just warn
-					if(responseID.empty() && r->code >= 500 && r->code < 600) {
-						sev = SevWarnAlways;
-					}
+				// If the response code is 5xx (server error) then a response ID is not expected
+				// so a missing id will be ignored but a mismatching id will still be an error.
+				bool serverError = r->code >= 500 && r->code < 600;
+
+				// If request/response IDs do not match and either this is not a server error
+				// or it is but the response ID is not empty then log an error.
+				if(requestID != responseID && (!serverError || !responseID.empty()) ) {
+					err = http_bad_request_id();
 
-					TraceEvent(sev, "HTTPRequestFailedIDMismatch")
+					TraceEvent(SevError, "HTTPRequestFailedIDMismatch")
 						.detail("DebugID", conn->getDebugID())
 						.detail("RemoteAddress", conn->getPeerAddress())
 						.detail("Verb", verb)
@@ -433,6 +433,7 @@ namespace HTTP {
 			return r;
 		} catch(Error &e) {
 			double elapsed = timer() - send_start;
+			// A bad_request_id error would have already been logged in verbose mode before err is thrown above.
 			if(CLIENT_KNOBS->HTTP_VERBOSE_LEVEL > 0 && e.code() != error_code_http_bad_request_id) {
 				printf("[%s] HTTP *ERROR*=%s early=%d, time=%fs %s %s contentLen=%d [%d out]\n",
 					conn->getDebugID().toString().c_str(), e.name(), earlyResponse, elapsed, verb.c_str(), resource.c_str(), contentLen, total_sent);