impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Brown (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4407: Move Impala setup procedures to main repo
Date Mon, 07 Aug 2017 17:13:53 GMT
Michael Brown has posted comments on this change.

Change subject: IMPALA-4407: Move Impala setup procedures to main repo
......................................................................


Patch Set 3:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/7587/3/bin/bootstrap_development.sh
File bin/bootstrap_development.sh:

PS3, Line 90: # IMPALA-3932, IMPALA-3926
            : SET_LD_LIBRARY_PATH='export LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu/${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}'
            : echo "$SET_LD_LIBRARY_PATH" >> ~/.bashrc
            : eval "$SET_LD_LIBRARY_PATH"
1. Is this needed for Ubuntu 14? I don't do this on my personal Ubuntu 14 system, and IMPALA-3932
says "This works in my experience if the system version is newer than the toolchain version."
2. If you run this script again and again it'll keep appending to .bashrc.


PS3, Line 96: sudo -u postgres psql -c "CREATE ROLE hiveuser LOGIN PASSWORD 'password';" postgres
If you run this script twice, this line always fails. Could this be enhanced to succeed if
the ROLE already exists?


PS3, Line 115: echo "NoHostAuthenticationForLocalhost yes" >> ~/.ssh/config
It might be polite to have a prompt for this since it's a security change.


PS3, Line 118: echo "127.0.0.1 $(hostname -s) $(hostname)" | sudo tee -a /etc/hosts
             : sudo sed -i 's/127.0.1.1/127.0.0.1/g' /etc/hosts
This will keep appending to /etc/hosts if you run the script more than once.


PS3, Line 124: echo "* - nofile 1048576" | sudo tee -a /etc/security/limits.conf
1. This keeps appending to limits.conf
2. Should this be changed to be restricted only to the current $USER?


PS3, Line 131: git clone https://github.com/cloudera/impala-lzo.git
             : ln -s impala-lzo Impala-lzo
             : git clone https://github.com/cloudera/hadoop-lzo.git
This fails if you run the script twice. Could you add -d tests here similar for what you do
with Impala?


PS3, Line 139: if [[ $VERSION = "14.04" ]]
             : then
             :   unset LD_LIBRARY_PATH
             : fi
Oh, this sort of answers my question above. What about when the user creates a new shell session?
He'll have sourced the modified .bashrc that sets LD_LIBRARY_PATH.


-- 
To view, visit http://gerrit.cloudera.org:8080/7587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I728abfa806ecd9461dfb443278c2a464714d984c
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message