Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/8570 )
Change subject: WIP: tls_socket: properly handle temporary socket errors in Writev ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8570/1/src/kudu/security/tls_socket.cc File src/kudu/security/tls_socket.cc: http://gerrit.cloudera.org:8080/#/c/8570/1/src/kudu/security/tls_socket.cc@99 PS1, Line 99: if (total_written > 0 && Socket::IsTemporarySocketError(write_status.posix_code())) { : return Status::OK(); : } > How do you know it will call it with the same length of 10? I think the code as it is today will always call with the same length. The edge case is if a partial write happens. But if Writev() does a partial write of an iovec/slice, it returns back to the caller to adjust the offset within that slice: https://github.com/apache/kudu/blob/f9a6a1b2d15feb8e2c044f20248774105a3a62bf/src/kudu/security/tls_socket.cc#L87 So there wouldn't be a case where writev() calls SSL_write() with a different length after a failure. But I agree that's not in the contract and it's best not to depend on implementation details that could change at any point, which may invalidate this fix. -- To view, visit http://gerrit.cloudera.org:8080/8570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If797f220f42bfb2e6f452b66f15e7a758e883472 Gerrit-Change-Number: 8570 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Thu, 16 Nov 2017 19:49:33 +0000 Gerrit-HasComments: Yes