Re: Review Request 55401: There are two questions in the sqoop_server_classpath_set function of the sqoop.sh. The sqoop2 start failure when using the same user with hadoop.

[Attila Szabo <ma...@apache.org>]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55401/#review166564
-----------------------------------------------------------



Hey Qiang,

Thanks for opening this ticket and review, we as the Sqoop community greatly appreciate you'd like to improve the quality of the product.

Although after I've reviewed your proposed changes I'm still uncerstain why these changes would be neccessary.

As I can see the current solution is even more flexible compared to the one you're advising (so it gives the possibility to the user to set those path env variables one by one, and of course it has a meaningful fallback mechanism).

Could you please provide me a bit more highlight why you'd like to do these changes?

Thanks,
Attila

- Attila Szabo


On Feb. 9, 2017, 7:16 a.m., Qiang Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55401/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2017, 7:16 a.m.)
> 
> 
> Review request for Sqoop, Andrew Bayer, Abraham Elmahrek, Abhijeet Gaikwad, Abraham Fine, Ahmed Radwan, Arvind Prabhakar, Cheolsoo Park, Colin Ma, David Robson, Dian Fu, Gwen Shapira, Hari Shreedharan, Jarek Cecho, Jonathan Hsieh, Kathleen Ting, Attila Szabo, Mengwei Ding, Olivier Lamy, and Roman Shaposhnik.
> 
> 
> Bugs: SQOOP-3102
>     https://issues.apache.org/jira/browse/SQOOP-3102
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> $HADOOP_YARN_HOME and $HADOOP_HDFS_HOME were used as following in start-all.sh of the hadoop:
> # start hdfs daemons if hdfs is present
> if [ -f "${HADOOP_HDFS_HOME}"/sbin/start-dfs.sh ]; then
>   "${HADOOP_HDFS_HOME}"/sbin/start-dfs.sh --config $HADOOP_CONF_DIR
> fi
> # start yarn daemons if yarn is present
> if [ -f "${HADOOP_YARN_HOME}"/sbin/start-yarn.sh ]; then
>   "${HADOOP_YARN_HOME}"/sbin/start-yarn.sh --config $HADOOP_CONF_DIR
> fi
> Based on above logic the $HADOOP_YARN_HOME and $HADOOP_HDFS_HOME must equal to $HADOOP_HOME. And they must be set as environment variable.
> 
> In sqoop2 they were used as following in sqoop_server_classpath_set function for sqoop.sh file.
> HADOOP_COMMON_HOME=${HADOOP_COMMON_HOME:-${HADOOP_HOME}/share/hadoop/common}
> HADOOP_HDFS_HOME=${HADOOP_HDFS_HOME:-${HADOOP_HOME}/share/hadoop/hdfs}
> HADOOP_MAPRED_HOME=${HADOOP_MAPRED_HOME:-${HADOOP_HOME}/share/hadoop/mapreduce}
> HADOOP_YARN_HOME=${HADOOP_YARN_HOME:-${HADOOP_HOME}/share/hadoop/yarn}
> In above logic the HADOOP_HDFS_HOME will equal to HADOOP_HOME when HADOOP_HDFS_HOME equal to $HADOOP_HOME in environment variable . The HADOOP_YARN_HOME will equal to HADOOP_HOME when HADOOP_YARN_HOME was set to $HADOOP_HOME in environment variable also.
> As a result, sqoop2 start failure.
> We shouldn't only check whether HADOOP_HDFS_HOME is empty, but also directly set HADOOP_HDFS_HOME=${HADOOP_HOME}/share/hadoop/hdfs and HADOOP_YARN_HOME={HADOOP_HOME}/share/hadoop/yarn in sqoop_server_classpath_set function for sqoop.sh file.
> 
> 
> Diffs
> -----
> 
>   dist/src/main/bin/sqoop.sh 3b2c6e1 
> 
> Diff: https://reviews.apache.org/r/55401/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qiang Zhang
> 
>