Henry Robinson has posted comments on this change.

Change subject: IMPALA-2888: StatestoreSslTest.SmokeTest failed
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/2630/2/be/src/statestore/statestore-test.cc
File be/src/statestore/statestore-test.cc:

Line 45: ASSERT_FALSE
ASSERT_NE?


http://gerrit.cloudera.org:8080/#/c/2630/2/be/src/testutil/in-process-servers.cc
File be/src/testutil/in-process-servers.cc:

Line 43: int backend_port = p, subscriber_port = ++p, webserver_port = ++p,
       :         beeswax_port = ++p, hs2_port = ++p;
won't this hit the same problem? you choose one ephemeral port, and then this 
test uses four others that haven't been checked. Is it cheap enough to call 
FindUnusedEphemeralPort() each time?


http://gerrit.cloudera.org:8080/#/c/2630/2/be/src/util/network-util.cc
File be/src/util/network-util.cc:

Line 147: std::
remove std::


Line 153: (char*)
reinterpret_cast


Line 164:   return -1;
you probably need to close(sockfd) here as well.


http://gerrit.cloudera.org:8080/#/c/2630/2/be/src/util/network-util.h
File be/src/util/network-util.h:

Line 52: .
say what this returns on error.


Line 52: /// Returns a ephemeral port that is unused when this function 
executes.
an


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I51b7a2c609d3a5ae6f7d32ef38db89efe71a882b
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-HasComments: Yes

Reply via email to