Sailesh Mukil 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?
Done


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 th
Yes, you're right. This could hit the same problem. Creating a server socket 
isn't very expensive. So it should be okay to call it 5 times.


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::
Done


Line 153: (char*)
> reinterpret_cast
Done


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


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

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


Line 52: .
> say what this returns on error.
Done


-- 
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-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-HasComments: Yes

Reply via email to