Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12271 )
Change subject: IMPALA-7999: clean up start-*d.sh scripts ...................................................................... Patch Set 7: (8 comments) http://gerrit.cloudera.org:8080/#/c/12271/6/bin/start-daemon.sh File bin/start-daemon.sh: http://gerrit.cloudera.org:8080/#/c/12271/6/bin/start-daemon.sh@31 PS6, Line 31: sanitis > Is this supposed to be sanitizer? Done http://gerrit.cloudera.org:8080/#/c/12271/6/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/12271/6/bin/start-impala-cluster.py@160 PS6, Line 160: java_tool_options = "" > The start-* scripts put the JAVA_TOOL_OPTIONS from the env at the end in ca Yeah I originally preserved that behaviour but realised it was redundant with setting -jvm_args http://gerrit.cloudera.org:8080/#/c/12271/6/bin/start-impala-cluster.py@163 PS6, Line 163: suspend=n > This is getting rid of the suspend=y option. I can't think of a time when i I don't really use the debugger functionality extensively and it doesn't seem like it was possible to set this from start-impala-cluster.py, so I don't know how likely it is that it was in use. I figured in some of these cases it was better to remove the functionality and add it back if someone noticed it missing rather than trying to port everything. http://gerrit.cloudera.org:8080/#/c/12271/6/bin/start-impala-cluster.py@372 PS6, Line 372: > Nit: stray line Done http://gerrit.cloudera.org:8080/#/c/12271/6/tests/common/impala_cluster.py File tests/common/impala_cluster.py: http://gerrit.cloudera.org:8080/#/c/12271/6/tests/common/impala_cluster.py@493 PS6, Line 493: output_file=None > Do we use the tool_prefix anywhere? I see that this replaces "-perf" and "- Yeah I guess it isn't really usable. I'll just remove it for now. I don't think it's too hard to add back in if someone misses it. http://gerrit.cloudera.org:8080/#/c/12271/6/tests/common/impala_cluster.py@497 PS6, Line 497: > Nit: stray "/" ? Done http://gerrit.cloudera.org:8080/#/c/12271/6/tests/common/impala_cluster.py@498 PS6, Line 498: > One thing I noticed is that gdb would need to run with the "--args" option Removed tool_prefix. http://gerrit.cloudera.org:8080/#/c/12271/6/tests/custom_cluster/test_redaction.py File tests/custom_cluster/test_redaction.py: http://gerrit.cloudera.org:8080/#/c/12271/6/tests/custom_cluster/test_redaction.py@95 PS6, Line 95: -audit_event_log_dir=%s : -profile_log_dir=%s : -redaction_rules_file=%s : -vmodule=%s""" > Nit: line up the '-' Done. This is why I don't like fancy vertical alignment in code - it's just fussy to change it. -- To view, visit http://gerrit.cloudera.org:8080/12271 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib67444fd4def8da119db5d3a0832ef1de15b068b Gerrit-Change-Number: 12271 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 04 Feb 2019 13:59:33 +0000 Gerrit-HasComments: Yes
