You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Brown (Code Review)" <ge...@cloudera.org> on 2019/02/07 03:29:41 UTC
[Impala-ASF-CR] IMPALA-8171: minicluster: use exec builtin to start Impala daemons
Michael Brown has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12391
Change subject: IMPALA-8171: minicluster: use exec builtin to start Impala daemons
......................................................................
IMPALA-8171: minicluster: use exec builtin to start Impala daemons
In IMPALA-7779 bin/start-daemon.sh was written to unify the place where
Impala daemons are started. It had a side-effect of keeping itself alive
as a parent process of the actual daemon. This broke the
tests.comparison.cluster abstraction used by the stress test and others
in that the process detection logic was finding false positives.
The old bin/start-*.sh scripts used the exec builtin to call their
daemons. Resurrect this as a simple fix; it also makes the ps list
shorter. Add a system test to catch this in the future.
Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
---
M bin/start-daemon.sh
M tests/infra/test_stress_infra.py
2 files changed, 14 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/12391/1
--
To view, visit http://gerrit.cloudera.org:8080/12391
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
Gerrit-Change-Number: 12391
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown <mi...@apache.org>
[Impala-ASF-CR] IMPALA-8171: minicluster: use exec builtin to start Impala daemons
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12391 )
Change subject: IMPALA-8171: minicluster: use exec builtin to start Impala daemons
......................................................................
Patch Set 1: Code-Review+2
I missed that subtlety of the original script. Thanks for catching.
--
To view, visit http://gerrit.cloudera.org:8080/12391
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
Gerrit-Change-Number: 12391
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 17:04:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8171: minicluster: use exec builtin to start Impala daemons
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/12391 )
Change subject: IMPALA-8171: minicluster: use exec builtin to start Impala daemons
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12391/1/bin/start-daemon.sh
File bin/start-daemon.sh:
http://gerrit.cloudera.org:8080/#/c/12391/1/bin/start-daemon.sh@33
PS1, Line 33: exec "$@"
I chose this because I didn't see any comments in https://gerrit.cloudera.org/#/c/12271/ about exec being intentionally omitted. If it turns out exec has a negative consequence and you don't want it here, I can fix this in a different way.
--
To view, visit http://gerrit.cloudera.org:8080/12391
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
Gerrit-Change-Number: 12391
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 16:10:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8171: minicluster: use exec builtin to start Impala daemons
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12391 )
Change subject: IMPALA-8171: minicluster: use exec builtin to start Impala daemons
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12391
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
Gerrit-Change-Number: 12391
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 17:49:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8171: minicluster: use exec builtin to start Impala daemons
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/12391 )
Change subject: IMPALA-8171: minicluster: use exec builtin to start Impala daemons
......................................................................
IMPALA-8171: minicluster: use exec builtin to start Impala daemons
In IMPALA-7779 bin/start-daemon.sh was written to unify the place where
Impala daemons are started. It had a side-effect of keeping itself alive
as a parent process of the actual daemon. This broke the
tests.comparison.cluster abstraction used by the stress test and others
in that the process detection logic was finding false positives.
The old bin/start-*.sh scripts used the exec builtin to call their
daemons. Resurrect this as a simple fix; it also makes the ps list
shorter. Add a system test to catch this in the future.
Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
Reviewed-on: http://gerrit.cloudera.org:8080/12391
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M bin/start-daemon.sh
M tests/infra/test_stress_infra.py
2 files changed, 14 insertions(+), 1 deletion(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12391
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
Gerrit-Change-Number: 12391
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8171: minicluster: use exec builtin to start Impala daemons
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12391 )
Change subject: IMPALA-8171: minicluster: use exec builtin to start Impala daemons
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2017/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12391
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
Gerrit-Change-Number: 12391
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 04:11:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8171: minicluster: use exec builtin to start Impala daemons
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12391 )
Change subject: IMPALA-8171: minicluster: use exec builtin to start Impala daemons
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3734/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12391
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
Gerrit-Change-Number: 12391
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 17:49:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8171: minicluster: use exec builtin to start Impala daemons
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12391 )
Change subject: IMPALA-8171: minicluster: use exec builtin to start Impala daemons
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12391
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34ac8ce964f0c56287cee01360a4ba889f9568d7
Gerrit-Change-Number: 12391
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@apache.org>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 21:48:06 +0000
Gerrit-HasComments: No