You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2018/06/14 22:40:57 UTC
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10726
Change subject: Fail cleanly when in process server can't bind
......................................................................
Fail cleanly when in process server can't bind
This doesn't solve IMPALA-7151 but makes the failure cleaner
because there is no crash.
Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
---
M be/src/exprs/expr-test.cc
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-test.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
5 files changed, 26 insertions(+), 27 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/10726/1
--
To view, visit http://gerrit.cloudera.org:8080/10726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Gerrit-Change-Number: 10726
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10726 )
Change subject: Fail cleanly when in process server can't bind
......................................................................
Patch Set 1: Code-Review+1
I thought a bit about not too intrusive ways to reduce the window where a port number is already decided but the port itself is unbound. A static/global port number->socket fd map could be created, and FindUnusedEphemeralPort() could add the fd to this map instead of closing it. Another static function like "ClosePortIfReserved()" could be called as close as possible to the place where the port is bound to socket by the actual server. This function would be a noop if the port is not in the map, so caller would not have to know if the port was "reserved" or not.
--
To view, visit http://gerrit.cloudera.org:8080/10726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Gerrit-Change-Number: 10726
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Jun 2018 12:11:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10726 )
Change subject: Fail cleanly when in process server can't bind
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2710/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/10726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Gerrit-Change-Number: 10726
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 22:03:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10726 )
Change subject: Fail cleanly when in process server can't bind
......................................................................
Patch Set 2: Code-Review+2
carry
--
To view, visit http://gerrit.cloudera.org:8080/10726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Gerrit-Change-Number: 10726
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 22:03:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10726 )
Change subject: Fail cleanly when in process server can't bind
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10726/1/be/src/testutil/in-process-servers.h
File be/src/testutil/in-process-servers.h:
http://gerrit.cloudera.org:8080/#/c/10726/1/be/src/testutil/in-process-servers.h@55
PS1, Line 55: /// forwarded to the ExecEnv.
can you specify whether *server is set only when OK is returned, or if it might be set even if an error is returned.
--
To view, visit http://gerrit.cloudera.org:8080/10726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Gerrit-Change-Number: 10726
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 21:59:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Sailesh Mukil, Csaba Ringhofer, Dan Hecht,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10726
to look at the new patch set (#2).
Change subject: Fail cleanly when in process server can't bind
......................................................................
Fail cleanly when in process server can't bind
This doesn't solve IMPALA-7151 but makes the failure cleaner
because there is no crash.
Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
---
M be/src/exprs/expr-test.cc
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-test.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
5 files changed, 30 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/10726/2
--
To view, visit http://gerrit.cloudera.org:8080/10726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Gerrit-Change-Number: 10726
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10726 )
Change subject: Fail cleanly when in process server can't bind
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10726/1/be/src/testutil/in-process-servers.h
File be/src/testutil/in-process-servers.h:
http://gerrit.cloudera.org:8080/#/c/10726/1/be/src/testutil/in-process-servers.h@55
PS1, Line 55: /// forwarded to the ExecEnv.
> can you specify whether *server is set only when OK is returned, or if it m
Done
--
To view, visit http://gerrit.cloudera.org:8080/10726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Gerrit-Change-Number: 10726
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 22:02:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10726 )
Change subject: Fail cleanly when in process server can't bind
......................................................................
Fail cleanly when in process server can't bind
This doesn't solve IMPALA-7151 but makes the failure cleaner
because there is no crash.
Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Reviewed-on: http://gerrit.cloudera.org:8080/10726
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/exprs/expr-test.cc
M be/src/service/session-expiry-test.cc
M be/src/statestore/statestore-test.cc
M be/src/testutil/in-process-servers.cc
M be/src/testutil/in-process-servers.h
5 files changed, 30 insertions(+), 29 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/10726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Gerrit-Change-Number: 10726
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10726 )
Change subject: Fail cleanly when in process server can't bind
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Gerrit-Change-Number: 10726
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Jun 2018 01:39:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] Fail cleanly when in process server can't bind
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10726 )
Change subject: Fail cleanly when in process server can't bind
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I376a2aa559f4b5cf3b96fa3465520e9983ecec4b
Gerrit-Change-Number: 10726
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 22:03:17 +0000
Gerrit-HasComments: No