You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2018/01/10 21:39:56 UTC

[kudu-CR] [tls socket-test] fix test to run smoothly at kernel 4.4.x

Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8996


Change subject: [tls_socket-test] fix test to run smoothly at kernel 4.4.x
......................................................................

[tls_socket-test] fix test to run smoothly at kernel 4.4.x

Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
---
M src/kudu/security/tls_socket-test.cc
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/8996/1
-- 
To view, visit http://gerrit.cloudera.org:8080/8996
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
Gerrit-Change-Number: 8996
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [tls socket-test] fix test to pass on Ubuntu 16.04

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8996

to look at the new patch set (#4).

Change subject: [tls_socket-test] fix test to pass on Ubuntu 16.04
......................................................................

[tls_socket-test] fix test to pass on Ubuntu 16.04

Prior to this fix, the TlsSocketTest.TestNonBlockingWritev test failed
on Ubuntu 16.04 (kernel 4.4.0).  After some investigation, it turned
out that setting socket send buffer size for something lower than the
MTU size (which is 64kB for localhost) triggered the delayed
acknowledgement logic.  Since the test passes big chunks of data
(32MB on every iteration), with 40ms delay for almost every 16KB sent,
the test eventually timed out.  I verified that disabling the delayed
TCP ack for TLS sockets fixed the issue.

I think that the proper fix is to remove the custom setting for the
socket send buffer: a 32MB chunk of data is big enough to not fit into
the socket buffer of the default size.

Also, it seems this issue is not going to bite us in real life since
we don't set the size of the socket send buffer anywhere.

NOTE: however, some mystery is left since the failing test would pass
      if running under strace.

Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
---
M src/kudu/security/tls_socket-test.cc
1 file changed, 0 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/8996/4
-- 
To view, visit http://gerrit.cloudera.org:8080/8996
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
Gerrit-Change-Number: 8996
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [tls socket-test] fix test to run smoothly at kernel 4.4.x

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8996 )

Change subject: [tls_socket-test] fix test to run smoothly at kernel 4.4.x
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8996/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8996/1//COMMIT_MSG@7
PS1, Line 7: [tls_socket-test] fix test to run smoothly at kernel 4.4.x
can you summarize briefly our findings here? ie that it appears that, if the socket buffer is less than the MTU size (which is 64kb for localhost) then delayed ack ends up kicking in and limiting us to one packet every 40ms, causing the test to time out?



-- 
To view, visit http://gerrit.cloudera.org:8080/8996
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
Gerrit-Change-Number: 8996
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 11 Jan 2018 01:13:23 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tls socket-test] fix test to pass on Ubuntu 16.04

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8996 )

Change subject: [tls_socket-test] fix test to pass on Ubuntu 16.04
......................................................................

[tls_socket-test] fix test to pass on Ubuntu 16.04

Prior to this fix, the TlsSocketTest.TestNonBlockingWritev test failed
on Ubuntu 16.04 (kernel 4.4.0).  After some investigation, it turned
out that setting socket send buffer size for something lower than the
MTU size (which is 64kB for localhost) triggered the delayed
acknowledgement logic.  Since the test passes big chunks of data
(32MB on every iteration), with 40ms delay for almost every 16KB sent,
the test eventually timed out.  I verified that disabling the delayed
TCP ack for TLS sockets fixed the issue.

I think that the proper fix is to remove the custom setting for the
socket send buffer: a 32MB chunk of data is big enough to not fit into
the socket buffer of the default size.

Also, it seems this issue is not going to bite us in real life since
we don't set the size of the socket send buffer anywhere.

NOTE: however, some mystery is left since the failing test would pass
      if running under strace.

Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
Reviewed-on: http://gerrit.cloudera.org:8080/8996
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/security/tls_socket-test.cc
1 file changed, 0 insertions(+), 4 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Todd Lipcon: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/8996
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
Gerrit-Change-Number: 8996
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [tls socket-test] fix test to pass at Ubuntu 16.04

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8996

to look at the new patch set (#2).

Change subject: [tls_socket-test] fix test to pass at Ubuntu 16.04
......................................................................

[tls_socket-test] fix test to pass at Ubuntu 16.04

Prior to this fix, the TlsSocketTest.TestNonBlockingWritev test failed
on Ubuntu 16.04 (kernel 4.4.0).  After some investigation, it turned
out that setting socket send buffer size for something lower than MTU
triggers the delayed acknowledgement logic to send ack for received
data after 40ms timeout.  Since the test passes big chunks of data
(32MB on every iteration), with 40ms delay for almost every 16KB sent,
the test eventually timed out.  I verified that disabling the delayed
TCP ack for TLS sockets fixed the issue.

I think that the proper fix is to remove the custom setting for the
socket send buffer: a 32MB chunk of data is big enough to not fit into
the socket buffer of the default size.

Also, it seems this issue is not going to bite us in real life since
we don't set the size of the socket send buffer anywhere.

NOTE: however, some mystery is left since the failing test would pass
      if running under strace.

Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
---
M src/kudu/security/tls_socket-test.cc
1 file changed, 0 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/8996/2
-- 
To view, visit http://gerrit.cloudera.org:8080/8996
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
Gerrit-Change-Number: 8996
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [tls socket-test] fix test to pass at Ubuntu 16.04

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/8996

to look at the new patch set (#3).

Change subject: [tls_socket-test] fix test to pass at Ubuntu 16.04
......................................................................

[tls_socket-test] fix test to pass at Ubuntu 16.04

Prior to this fix, the TlsSocketTest.TestNonBlockingWritev test failed
on Ubuntu 16.04 (kernel 4.4.0).  After some investigation, it turned
out that setting socket send buffer size for something lower than the
MTU size (which is 64kB for localhost) triggers the delayed
acknowledgement logic to send ack for the received data after 40ms
timeout.  Since the test passes big chunks of data
(32MB on every iteration), with 40ms delay for almost every 16KB sent,
the test eventually timed out.  I verified that disabling the delayed
TCP ack for TLS sockets fixed the issue.

I think that the proper fix is to remove the custom setting for the
socket send buffer: a 32MB chunk of data is big enough to not fit into
the socket buffer of the default size.

Also, it seems this issue is not going to bite us in real life since
we don't set the size of the socket send buffer anywhere.

NOTE: however, some mystery is left since the failing test would pass
      if running under strace.

Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
---
M src/kudu/security/tls_socket-test.cc
1 file changed, 0 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/8996/3
-- 
To view, visit http://gerrit.cloudera.org:8080/8996
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
Gerrit-Change-Number: 8996
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [tls socket-test] fix test to pass on Ubuntu 16.04

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8996 )

Change subject: [tls_socket-test] fix test to pass on Ubuntu 16.04
......................................................................


Patch Set 4: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/8996
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94170d5bdbe17a5952a82faf74d8b868b42460aa
Gerrit-Change-Number: 8996
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 12 Jan 2018 05:28:45 +0000
Gerrit-HasComments: No