Skip to content

Conversation

@morningman
Copy link
Contributor

@morningman morningman commented Mar 29, 2023

Proposed changes

Issue Number: close #xxx

Problem summary

  1. Introduce hadoop libhdfs
  2. For Linux-X86 platform, use the hadoop libhdfs
  3. For other platform, use libhdfs3, because currently we don't have hadoop libhdfs binary for other platform

Co-authored-by: adonis0147 adonis0147@gmail.com

Checklist(Required)

  • Does it affect the original behavior
  • Has unit tests been added
  • Has document been added or modified
  • Does it need to update dependencies
  • Is this PR support rollback (If NO, please explain WHY)

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@github-actions
Copy link
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors

'shellcheck ' returned error 1 finding the following syntactical issues:

----------

In bin/start_be.sh line 100:
export CLASSPATH="${DORIS_HOME}/conf/:$DORIS_CLASSPATH"
                                      ^--------------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
export CLASSPATH="${DORIS_HOME}/conf/:${DORIS_CLASSPATH}"


In bin/start_be.sh line 262:
        export LD_LIBRARY_PATH=$JAVA_HOME/jre/lib/$jvm_arch/server:$JAVA_HOME/jre/lib/$jvm_arch:$LD_LIBRARY_PATH
                               ^--------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                  ^-------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                                   ^--------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                                                      ^-------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                                                                ^--------------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
        export LD_LIBRARY_PATH=${JAVA_HOME}/jre/lib/${jvm_arch}/server:${JAVA_HOME}/jre/lib/${jvm_arch}:${LD_LIBRARY_PATH}


In bin/start_be.sh line 263:
        export LD_LIBRARY_PATH=$DORIS_HOME/lib/hadoop_hdfs/native:$LD_LIBRARY_PATH
                               ^---------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                                  ^--------------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
        export LD_LIBRARY_PATH=${DORIS_HOME}/lib/hadoop_hdfs/native:${LD_LIBRARY_PATH}


In bin/start_be.sh line 264:
        export LIBHDFS_OPTS="${JAVA_OPTS}"
                             ^----------^ SC2154 (warning): JAVA_OPTS is referenced but not assigned.


In bin/start_be.sh line 269:
echo "CLASSPATH: ${CLASSPATH}\n"
     ^-------------------------^ SC2028 (info): echo may not expand escape sequences. Use printf.


In bin/start_be.sh line 270:
echo "LD_LIBRARY_PATH: ${LD_LIBRARY_PATH}\n"
     ^-- SC2028 (info): echo may not expand escape sequences. Use printf.


In bin/start_be.sh line 271:
echo "LIBHDFS_OPTS: ${LIBHDFS_OPTS}\n"
     ^-- SC2028 (info): echo may not expand escape sequences. Use printf.

For more information:
  https://www.shellcheck.net/wiki/SC2154 -- JAVA_OPTS is referenced but not a...
  https://www.shellcheck.net/wiki/SC2028 -- echo may not expand escape sequen...
  https://www.shellcheck.net/wiki/SC2250 -- Prefer putting braces around vari...
----------

You can address the above issues in one of three ways:
1. Manually correct the issue in the offending shell script;
2. Disable specific issues by adding the comment:
  # shellcheck disable=NNNN
above the line that contains the issue, where NNNN is the error code;
3. Add '-e NNNN' to the SHELLCHECK_OPTS setting in your .yml action file.



shfmt errors
'shfmt ' found no issues.

@github-actions
Copy link
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors

'shellcheck ' returned error 1 finding the following syntactical issues:

----------

In bin/start_be.sh line 101:
export CLASSPATH="${DORIS_HOME}/conf/:$DORIS_CLASSPATH"
                                      ^--------------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
export CLASSPATH="${DORIS_HOME}/conf/:${DORIS_CLASSPATH}"


In bin/start_be.sh line 257:
if [[ -z $JAVA_OPTS ]]; then
         ^--------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
if [[ -z ${JAVA_OPTS} ]]; then


In bin/start_be.sh line 260:
    JAVA_OPTS="-Xmx1024m -DlogPath=$DORIS_HOME/log/jni.log -Xloggc:$DORIS_HOME/log/be.gc.log.$CUR_DATE -Dsun.java.command=DorisBE -XX:-CriticalJNINatives"
                                   ^---------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                                   ^---------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                                                             ^-------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
    JAVA_OPTS="-Xmx1024m -DlogPath=${DORIS_HOME}/log/jni.log -Xloggc:${DORIS_HOME}/log/be.gc.log.${CUR_DATE} -Dsun.java.command=DorisBE -XX:-CriticalJNINatives"

For more information:
  https://www.shellcheck.net/wiki/SC2250 -- Prefer putting braces around vari...
----------

You can address the above issues in one of three ways:
1. Manually correct the issue in the offending shell script;
2. Disable specific issues by adding the comment:
  # shellcheck disable=NNNN
above the line that contains the issue, where NNNN is the error code;
3. Add '-e NNNN' to the SHELLCHECK_OPTS setting in your .yml action file.



shfmt errors
'shfmt ' found no issues.

@github-actions
Copy link
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors

'shellcheck ' returned error 1 finding the following syntactical issues:

----------

In bin/start_be.sh line 101:
export CLASSPATH="${DORIS_HOME}/conf/:$DORIS_CLASSPATH"
                                      ^--------------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
export CLASSPATH="${DORIS_HOME}/conf/:${DORIS_CLASSPATH}"


In bin/start_be.sh line 257:
if [[ -z $JAVA_OPTS ]]; then
         ^--------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
if [[ -z ${JAVA_OPTS} ]]; then


In bin/start_be.sh line 260:
    JAVA_OPTS="-Xmx1024m -DlogPath=$DORIS_HOME/log/jni.log -Xloggc:$DORIS_HOME/log/be.gc.log.$CUR_DATE -Dsun.java.command=DorisBE -XX:-CriticalJNINatives"
                                   ^---------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                                   ^---------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                                                             ^-------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
    JAVA_OPTS="-Xmx1024m -DlogPath=${DORIS_HOME}/log/jni.log -Xloggc:${DORIS_HOME}/log/be.gc.log.${CUR_DATE} -Dsun.java.command=DorisBE -XX:-CriticalJNINatives"

For more information:
  https://www.shellcheck.net/wiki/SC2250 -- Prefer putting braces around vari...
----------

You can address the above issues in one of three ways:
1. Manually correct the issue in the offending shell script;
2. Disable specific issues by adding the comment:
  # shellcheck disable=NNNN
above the line that contains the issue, where NNNN is the error code;
3. Add '-e NNNN' to the SHELLCHECK_OPTS setting in your .yml action file.



shfmt errors
'shfmt ' found no issues.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

options = (JavaVMOption*)calloc(no_args, sizeof(JavaVMOption));
options[0].optionString = const_cast<char*>(classpath.c_str());
java_opts = getenv("JAVA_OPTS");
if (java_opts != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: use nullptr [modernize-use-nullptr]

Suggested change
if (java_opts != NULL) {
if (java_opts != nullptr) {

@morningman
Copy link
Contributor Author

run buildall

1 similar comment
@morningman
Copy link
Contributor Author

run buildall

@morningman morningman changed the title [Draft](libhdfs) introduce hadoop libhdfs [refactor](libhdfs) introduce hadoop libhdfs Mar 30, 2023
Comment on lines 146 to 150

if (java_opts != nullptr) {
free(java_opts);
}
free(options);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to use RAII.

@morningman
Copy link
Contributor Author

run buildall

@github-actions
Copy link
Contributor

sh-checker report

To get the full details, please check in the job output.

shellcheck errors

'shellcheck ' returned error 1 finding the following syntactical issues:

----------

In bin/start_be.sh line 101:
export CLASSPATH="${DORIS_HOME}/conf/:$DORIS_CLASSPATH"
                                      ^--------------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
export CLASSPATH="${DORIS_HOME}/conf/:${DORIS_CLASSPATH}"


In bin/start_be.sh line 259:
if [[ -z $JAVA_OPTS ]]; then
         ^--------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
if [[ -z ${JAVA_OPTS} ]]; then


In bin/start_be.sh line 262:
    JAVA_OPTS="-Xmx1024m -DlogPath=$DORIS_HOME/log/jni.log -Xloggc:$DORIS_HOME/log/be.gc.log.$CUR_DATE -Dsun.java.command=DorisBE -XX:-CriticalJNINatives"
                                   ^---------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                                   ^---------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.
                                                                                             ^-------^ SC2250 (style): Prefer putting braces around variable references even when not strictly required.

Did you mean: 
    JAVA_OPTS="-Xmx1024m -DlogPath=${DORIS_HOME}/log/jni.log -Xloggc:${DORIS_HOME}/log/be.gc.log.${CUR_DATE} -Dsun.java.command=DorisBE -XX:-CriticalJNINatives"

For more information:
  https://www.shellcheck.net/wiki/SC2250 -- Prefer putting braces around vari...
----------

You can address the above issues in one of three ways:
1. Manually correct the issue in the offending shell script;
2. Disable specific issues by adding the comment:
  # shellcheck disable=NNNN
above the line that contains the issue, where NNNN is the error code;
3. Add '-e NNNN' to the SHELLCHECK_OPTS setting in your .yml action file.



shfmt errors
'shfmt ' found no issues.

@morningman
Copy link
Contributor Author

run buildall

@morningman
Copy link
Contributor Author

run buildall

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

adonis0147
adonis0147 previously approved these changes Mar 30, 2023
Copy link
Contributor

@adonis0147 adonis0147 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Mar 30, 2023
@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Mar 30, 2023
Copy link
Contributor

@adonis0147 adonis0147 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@morningman
Copy link
Contributor Author

run buildall

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Mar 30, 2023
@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@morningman morningman merged commit 7e61a85 into apache:master Mar 31, 2023
gnehil pushed a commit to gnehil/doris that referenced this pull request Apr 21, 2023
1. Introduce hadoop libhdfs 
2. For Linux-X86 platform, use the hadoop libhdfs
3. For other platform, use libhdfs3, because currently we don't have  hadoop libhdfs binary for other platform

Co-authored-by: adonis0147 <adonis0147@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. area/vectorization reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants