[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. IMPALA-7151: Rework ephemeral port assignment for be tests Instead of using FindUnusedEphemeralPort() to choose many ephemeral ports for in-process servers, we use Thrift's builtin wildcard ports, where it picks an ephemeral port when the configured port is 0. This should be more robust because there's no window where something else can steal our port. Only Thrift 0.9.2+ support the getPort() method required for this. This does not necessarily make all ImpalaServer functionality work with ports configured to 0, just enough to get expr-test and session-expiry-test to work. E.g. various strings and names get port 0 in them and query execution may not work end-to-end. Some tests still use FindUnusedEphemeralPort() to pick a single port at a time. We haven't seen that usage pattern to be flaky with any frequency and it appears to be more work to fix those, so I left them for now. Testing: Ran core tests. Cherry-picks: not for 2.x Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Reviewed-on: http://gerrit.cloudera.org:8080/10740 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/benchmarks/expr-benchmark.cc M be/src/exprs/expr-test.cc M be/src/rpc/rpc-mgr-kerberized-test.cc M be/src/rpc/rpc-mgr-test-base.h M be/src/rpc/rpc-mgr-test.cc M be/src/rpc/thrift-server-test.cc M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/scheduling/scheduler.cc M be/src/scheduling/scheduler.h M be/src/service/client-request-state.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/session-expiry-test.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M be/src/statestore/statestore-test.cc M be/src/testutil/in-process-servers.cc M be/src/testutil/in-process-servers.h M be/src/util/network-util.cc M be/src/util/network-util.h 23 files changed, 175 insertions(+), 128 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 20 Jun 2018 22:22:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 4: Checked with Anuj directly... -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 20 Jun 2018 19:06:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2717/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 20 Jun 2018 19:06:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 20 Jun 2018 19:06:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc@223 PS4, Line 223: heartbeat_address_.port = heartbeat_server_->port(); > Oh my bad. I completely misinterpreted the scope of the lock by looking at Np, it's better than you don't assume that I got it right. -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 20 Jun 2018 17:46:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 4: Code-Review+2 (1 comment) LGTM. Please also check with Anuj if he has any further comments. http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc@223 PS4, Line 223: heartbeat_address_.port = heartbeat_server_->port(); > Am I misreading something - isn't this within the same scoped block as excl Oh my bad. I completely misinterpreted the scope of the lock by looking at the wrong curly braces. -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 20 Jun 2018 17:43:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc@223 PS4, Line 223: heartbeat_address_.port = heartbeat_server_->port(); > Are we sure that setting this outside the scope of 'lock_' won't cause any Am I misreading something - isn't this within the same scoped block as exclusive_lock? -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 20 Jun 2018 17:40:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc File be/src/statestore/statestore-subscriber.cc: http://gerrit.cloudera.org:8080/#/c/10740/4/be/src/statestore/statestore-subscriber.cc@223 PS4, Line 223: heartbeat_address_.port = heartbeat_server_->port(); Are we sure that setting this outside the scope of 'lock_' won't cause any kind of race? I don't exactly understand the comment on L193-195 since Register() happens only after dropping the lock_, so I'm not able to make the call. -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 20 Jun 2018 17:05:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/rpc/thrift-server.cc File be/src/rpc/thrift-server.cc: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/rpc/thrift-server.cc@366 PS2, Line 366: TServerSocket > Are we sure that this change from TServerTransport to TServerSocket doesn't TServerSocket is a subclass of TServerTransport - I needed to use the subclass here for getPort() - https://github.com/apache/thrift/blob/0.9.3/lib/cpp/src/thrift/transport/TServerSocket.h#L116 So yeah, shouldn't limit us at all. http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/rpc/thrift-server.cc@452 PS2, Line 452: If port was 0 > nit: "If port_ was initially configured as 0..." I think I meant the word port, but referring to the variable is probably clearer. http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/runtime/exec-env.h File be/src/runtime/exec-env.h: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/runtime/exec-env.h@222 PS2, Line 222: TNetworkAddress configured_backend_address_; > Why make this public? I'm not sure why I did this... reverted. http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/scheduling/scheduler.h File be/src/scheduling/scheduler.h: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/scheduling/scheduler.h@96 PS2, Line 96: UpdateLocalBackendForBeTest > UpdateLocalBackendAddrForBeTest Done http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/service/impala-server.h@142 PS2, Line 142: ///respective service will not be started. > Update comment with new behavior about BE tests passing in port=0. Done http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/service/impala-server.cc@2070 PS2, Line 2070: >= > > 0 Thanks for catching this. http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/statestore/statestore-subscriber.h File be/src/statestore/statestore-subscriber.h: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/statestore/statestore-subscriber.h@190 PS2, Line 190: /// Address that the heartbeat service should be started on. Initialised in constructor, : /// updated in Start() with the actual port if the wildcard port 0 was specified. : TNetworkAddress heartbeat_address_; > Any reason this was moved down? Prefer leaving it where it was as surroundi It's protected by the lock now, since it's modified in start rather than constant for the lifetime of the object. It was only ever referenced under the lock before anyway. http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/util/network-util.cc File be/src/util/network-util.cc: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/util/network-util.cc@196 PS2, Line 196: server_address.sin_port = htons(port); > I think we can set this to 0 and bind to a random port instead of looping t Nice catch! I hadn't really thought about it. We talked out of band and my concern was that doing this might increase the risk of collisions. But it sounds like it probably makes things better because linux apparently allocates them pseudo-sequentially: https://eklitzke.org/binding-on-port-zero . That whole blog post is really relevant. I think you said you were going to look at doing it, but I can also do as a follow-on if needed. -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Wed, 20 Jun 2018 16:10:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Hello Sailesh Mukil, anujphadke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10740 to look at the new patch set (#3). Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. IMPALA-7151: Rework ephemeral port assignment for be tests Instead of using FindUnusedEphemeralPort() to choose many ephemeral ports for in-process servers, we use Thrift's builtin wildcard ports, where it picks an ephemeral port when the configured port is 0. This should be more robust because there's no window where something else can steal our port. Only Thrift 0.9.2+ support the getPort() method required for this. This does not necessarily make all ImpalaServer functionality work with ports configured to 0, just enough to get expr-test and session-expiry-test to work. E.g. various strings and names get port 0 in them and query execution may not work end-to-end. Some tests still use FindUnusedEphemeralPort() to pick a single port at a time. We haven't seen that usage pattern to be flaky with any frequency and it appears to be more work to fix those, so I left them for now. Testing: Ran core tests. Cherry-picks: not for 2.x Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 --- M be/src/benchmarks/expr-benchmark.cc M be/src/exprs/expr-test.cc M be/src/rpc/rpc-mgr-kerberized-test.cc M be/src/rpc/rpc-mgr-test-base.h M be/src/rpc/rpc-mgr-test.cc M be/src/rpc/thrift-server-test.cc M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/scheduling/scheduler.cc M be/src/scheduling/scheduler.h M be/src/service/client-request-state.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/session-expiry-test.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M be/src/statestore/statestore-test.cc M be/src/testutil/in-process-servers.cc M be/src/testutil/in-process-servers.h M be/src/util/network-util.cc M be/src/util/network-util.h 23 files changed, 175 insertions(+), 128 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/10740/3 -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
anujphadke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/util/network-util.cc File be/src/util/network-util.cc: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/util/network-util.cc@196 PS2, Line 196: server_address.sin_port = htons(port); I think we can set this to 0 and bind to a random port instead of looping through to find a random port? -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Tue, 19 Jun 2018 23:05:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 2: (7 comments) Thanks for doing this. This also sets up some of the code nicely for IMPALA-6013 which I'll be getting to soon. http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/rpc/thrift-server.cc File be/src/rpc/thrift-server.cc: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/rpc/thrift-server.cc@366 PS2, Line 366: TServerSocket Are we sure that this change from TServerTransport to TServerSocket doesn't handicap us from using TServerTransport APIs that we may find useful in the future? Or were we doing the wrong thing by using TServerTransport all along? http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/rpc/thrift-server.cc@452 PS2, Line 452: If port was 0 nit: "If port_ was initially configured as 0..." http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/runtime/exec-env.h File be/src/runtime/exec-env.h: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/runtime/exec-env.h@222 PS2, Line 222: TNetworkAddress configured_backend_address_; Why make this public? http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/scheduling/scheduler.h File be/src/scheduling/scheduler.h: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/scheduling/scheduler.h@96 PS2, Line 96: UpdateLocalBackendForBeTest UpdateLocalBackendAddrForBeTest http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/service/impala-server.h@142 PS2, Line 142: ///respective service will not be started. Update comment with new behavior about BE tests passing in port=0. http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/service/impala-server.cc@2070 PS2, Line 2070: >= > 0 http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/statestore/statestore-subscriber.h File be/src/statestore/statestore-subscriber.h: http://gerrit.cloudera.org:8080/#/c/10740/2/be/src/statestore/statestore-subscriber.h@190 PS2, Line 190: /// Address that the heartbeat service should be started on. Initialised in constructor, : /// updated in Start() with the actual port if the wildcard port 0 was specified. : TNetworkAddress heartbeat_address_; Any reason this was moved down? Prefer leaving it where it was as surrounding members are similar there. -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 19 Jun 2018 22:57:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 2: > Ping I'll have a first pass at this by EOD. -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 19 Jun 2018 20:03:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 2: Ping -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 19 Jun 2018 19:04:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. Patch Set 2: This is stacked on top of https://gerrit.cloudera.org/#/c/10726/ -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 18 Jun 2018 23:20:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7151: Rework ephemeral port assignment for be tests
Tim Armstrong has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/10740 ) Change subject: IMPALA-7151: Rework ephemeral port assignment for be tests .. IMPALA-7151: Rework ephemeral port assignment for be tests Instead of using FindUnusedEphemeralPort() to choose many ephemeral ports for in-process servers, we use Thrift's builtin wildcard ports, where it picks an ephemeral port when the configured port is 0. This should be more robust because there's no window where something else can steal our port. Only Thrift 0.9.2+ support the getPort() method required for this. This does not necessarily make all ImpalaServer functionality work with ports configured to 0, just enough to get expr-test and session-expiry-test to work. E.g. various strings and names get port 0 in them and query execution may not work end-to-end. Some tests still use FindUnusedEphemeralPort() to pick a single port at a time. We haven't seen that usage pattern to be flaky with any frequency and it appears to be more work to fix those, so I left them for now. Testing: Ran core tests. Cherry-picks: not for 2.x Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 --- M be/src/benchmarks/expr-benchmark.cc M be/src/exprs/expr-test.cc M be/src/rpc/rpc-mgr-kerberized-test.cc M be/src/rpc/rpc-mgr-test-base.h M be/src/rpc/rpc-mgr-test.cc M be/src/rpc/thrift-server-test.cc M be/src/rpc/thrift-server.cc M be/src/rpc/thrift-server.h M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/scheduling/scheduler.cc M be/src/scheduling/scheduler.h M be/src/service/client-request-state.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/session-expiry-test.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M be/src/statestore/statestore-test.cc M be/src/testutil/in-process-servers.cc M be/src/testutil/in-process-servers.h M be/src/util/network-util.cc M be/src/util/network-util.h 23 files changed, 169 insertions(+), 127 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/10740/2 -- To view, visit http://gerrit.cloudera.org:8080/10740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2fcc9243099f8249380ac5f01ac5eb67cba24cf5 Gerrit-Change-Number: 10740 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong
