[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14471 ) Change subject: Fix webserver's use of sq_printf/sq_write .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 17 Oct 2019 04:21:18 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14471 ) Change subject: Fix webserver's use of sq_printf/sq_write .. Fix webserver's use of sq_printf/sq_write Previously, the webserver used multiple calls to sq_printf and sq_write when sending most responses. This can lead to a bad interaction between Nagle's algorithm and TCP delayed acks which can add significant latency to RTT. This patch modifies the SendResponse() function to buffer the entire response and send it in a single sq_write call. Testing: - Ran all existing webserver tests. Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Reviewed-on: http://gerrit.cloudera.org:8080/14471 Reviewed-by: Adar Dembo Reviewed-by: Thomas Tauber-Marshall Tested-by: Impala Public Jenkins --- M be/src/util/webserver.cc 1 file changed, 16 insertions(+), 9 deletions(-) Approvals: Adar Dembo: Looks good to me, but someone else must approve Thomas Tauber-Marshall: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14471 ) Change subject: Fix webserver's use of sq_printf/sq_write .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4811/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 17 Oct 2019 00:41:49 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14471 ) Change subject: Fix webserver's use of sq_printf/sq_write .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5100/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 16 Oct 2019 23:59:55 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14471 ) Change subject: Fix webserver's use of sq_printf/sq_write .. Patch Set 2: Code-Review+2 carrying forward -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 16 Oct 2019 23:59:36 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14471 ) Change subject: Fix webserver's use of sq_printf/sq_write .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 16 Oct 2019 23:59:06 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14471 ) Change subject: Fix webserver's use of sq_printf/sq_write .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/14471/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14471/1//COMMIT_MSG@12 PS1, Line 12: This can lead to a bad interaction between Nagle's algorithm and TCP : delayed acks which can add significant latency to RTT. > This manifested in Kudu most noticeably in webserver-test, specifically in No, we don't have any such tests http://gerrit.cloudera.org:8080/#/c/14471/1/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/14471/1/be/src/util/webserver.cc@185 PS1, Line 185: // Buffer the output and send it in a single call to sq_write in order to avoid > Could add a comment here explaining why we're buffering in the ostringstrea Done -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 16 Oct 2019 23:58:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14471 ) Change subject: Fix webserver's use of sq_printf/sq_write .. Patch Set 1: Code-Review+2 Same comment as Adar about including a brief explanation, but LGTM -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 16 Oct 2019 23:57:00 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Hello Adar Dembo, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14471 to look at the new patch set (#2). Change subject: Fix webserver's use of sq_printf/sq_write .. Fix webserver's use of sq_printf/sq_write Previously, the webserver used multiple calls to sq_printf and sq_write when sending most responses. This can lead to a bad interaction between Nagle's algorithm and TCP delayed acks which can add significant latency to RTT. This patch modifies the SendResponse() function to buffer the entire response and send it in a single sq_write call. Testing: - Ran all existing webserver tests. Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f --- M be/src/util/webserver.cc 1 file changed, 16 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/14471/2 -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14471 ) Change subject: Fix webserver's use of sq_printf/sq_write .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4809/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 16 Oct 2019 23:43:17 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14471 ) Change subject: Fix webserver's use of sq_printf/sq_write .. Patch Set 1: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/14471/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14471/1//COMMIT_MSG@12 PS1, Line 12: This can lead to a bad interaction between Nagle's algorithm and TCP : delayed acks which can add significant latency to RTT. This manifested in Kudu most noticeably in webserver-test, specifically in those tests that issued many HTTP requests with connection reuse (i.e. after keep-alive was enabled). We saw a 40ms delay between each request. Are there any Impala tests that works similarly? http://gerrit.cloudera.org:8080/#/c/14471/1/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/14471/1/be/src/util/webserver.cc@185 PS1, Line 185: std::ostringstream oss; Could add a comment here explaining why we're buffering in the ostringstream. -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 16 Oct 2019 23:04:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Fix webserver's use of sq printf/sq write
Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14471 Change subject: Fix webserver's use of sq_printf/sq_write .. Fix webserver's use of sq_printf/sq_write Previously, the webserver used multiple calls to sq_printf and sq_write when sending most responses. This can lead to a bad interaction between Nagle's algorithm and TCP delayed acks which can add significant latency to RTT. This patch modifies the SendResponse() function to buffer the entire response and send it in a single sq_write call. Testing: - Ran all existing webserver tests. Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f --- M be/src/util/webserver.cc 1 file changed, 14 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/14471/1 -- To view, visit http://gerrit.cloudera.org:8080/14471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I343ae83d6324bc710e4cf96d66975a7c9694706f Gerrit-Change-Number: 14471 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall
