[KYUUBI #5987] Always export KYUUBI_HOME in `load-kyuubi-env.sh` #6004

pan3793 · 2024-01-22T06:31:03Z

🔍 Description

Issue References 🔗

This is a follow-up of #5987, after applying #5987, I found a regression, the Kyuubi Server Java process can not see KYUUBI_HOME env if we don't declare KYUUBI_HOME as an env var explicitly, this breaks the main resources jar discovery of the engine, and eventually fails the engine bootstrap.

Describe Your Solution 🔧

Restore export KYUUBI_HOME logic in load-kyuubi-env.sh

Types of changes 🔖

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Build and start Kyuubi Server.

export SPARK_HOME=/Users/chengpan/app/spark-3.5.0-bin-hadoop3
build/dist --spark-provided --flink-provided --hive-provided
dist/bin/kyuubi run

Open session to launch a Spark engine

dist/bin/beeline -u 'jdbc:hive2://0.0.0.0:10009/default'

Behavior Without This Pull Request ⚰️

A wrong kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar location was found.

/Users/chengpan/app/spark-3.5.0-bin-hadoop3/bin/spark-submit \
        ...
        --proxy-user chengpan /Users/chengpan/Projects/apache-kyuubi/dist/jars/externals/kyuubi-spark-sql-engine/target/kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar

Behavior With This Pull Request 🎉

The correct kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar location was found.

/Users/chengpan/app/spark-3.5.0-bin-hadoop3/bin/spark-submit \
        ...
        --proxy-user chengpan /Users/chengpan/Projects/apache-kyuubi/dist/externals/engines/spark/kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar

Checklist 📝

This patch was not authored or co-authored using Generative Tooling

Be nice. Be informative.

pan3793 · 2024-01-22T06:32:06Z

cc @Emor-nj, sorry I didn't spot this issue during the previous review, would you please take a look?

codecov-commenter · 2024-01-22T07:58:37Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (777b784) 61.11% compared to head (a4e40b6) 61.08%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #6004      +/-   ##
============================================
- Coverage     61.11%   61.08%   -0.03%     
  Complexity       23       23              
============================================
  Files           623      623              
  Lines         37060    37060              
  Branches       5024     5024              
============================================
- Hits          22648    22638      -10     
- Misses        11967    11975       +8     
- Partials       2445     2447       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Emor-nj · 2024-01-22T08:45:27Z

cc @Emor-nj, sorry I didn't spot this issue during the previous review, would you please take a look?

I think the reason is that when the Kyuubi service starts, the Kyuubi script(https://github.com/apache/kyuubi/blob/master/bin/kyuubi) does not declare KYUUBI_HOME as an environment variable. It is only passed as a context variable to load-kyuubi-env.sh. Therefore, although load-kyuubi-env.sh detects that KYUUBI_HOME has a value, it still does not declare it as an environment variable. As a result, when launching the Kyuubi service process, Java cannot retrieve KYUUBI_HOME from the system environment variables. Consequently, ProcBuilder(https://github.com/apache/kyuubi/blob/master/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala) cannot obtain the correct main resources JAR when constructing the spark-submit command.
The code in ProcBuilder is:
env.get(KYUUBI_HOME).toSeq .flatMap { p => Seq( Paths.get(p, "externals", "engines", shortName, jarName), Paths.get(p, "externals", module, "target", jarName)) } .find(Files.exists(_)).map(_.toAbsolutePath.toFile.getCanonicalPath)
env.get(KYUUBI_HOME) will be empty.

pan3793 · 2024-01-22T08:48:35Z

@Emor-nj yea, I have the same conclusion, do you have concerns about this fix?

Emor-nj · 2024-01-22T08:58:23Z

@Emor-nj yea, I have the same conclusion, do you have concerns about this fix?

No，it looks good to me.

# 🔍 Description ## Issue References 🔗 This is a follow-up of #5987, after applying #5987, I found a regression, the Kyuubi Server Java process can not see KYUUBI_HOME env if we don't declare KYUUBI_HOME as an env var explicitly, this breaks the main resources jar discovery of the engine, and eventually fails the engine bootstrap. ## Describe Your Solution 🔧 Restore `export KYUUBI_HOME` logic in `load-kyuubi-env.sh` ## Types of changes 🔖 - [x] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 Build and start Kyuubi Server. ``` export SPARK_HOME=/Users/chengpan/app/spark-3.5.0-bin-hadoop3 build/dist --spark-provided --flink-provided --hive-provided dist/bin/kyuubi run ``` Open session to launch a Spark engine ``` dist/bin/beeline -u 'jdbc:hive2://0.0.0.0:10009/default' ``` #### Behavior Without This Pull Request ⚰️ A wrong `kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar` location was found. ``` /Users/chengpan/app/spark-3.5.0-bin-hadoop3/bin/spark-submit \ ... --proxy-user chengpan /Users/chengpan/Projects/apache-kyuubi/dist/jars/externals/kyuubi-spark-sql-engine/target/kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar ``` #### Behavior With This Pull Request 🎉 The correct `kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar` location was found. ``` /Users/chengpan/app/spark-3.5.0-bin-hadoop3/bin/spark-submit \ ... --proxy-user chengpan /Users/chengpan/Projects/apache-kyuubi/dist/externals/engines/spark/kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar ``` --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes #6004 from pan3793/5987-followup. Closes #5987 a4e40b6 [Cheng Pan] [KYUUBI #5987] Always export KYUUBI_HOME in load-kyuubi-env.sh Authored-by: Cheng Pan <[email protected]> Signed-off-by: Cheng Pan <[email protected]> (cherry picked from commit ac388d1) Signed-off-by: Cheng Pan <[email protected]>

pan3793 · 2024-01-22T13:32:52Z

Thanks, merged to master/1.8

# 🔍 Description ## Issue References 🔗 This is a follow-up of apache#5987, after applying apache#5987, I found a regression, the Kyuubi Server Java process can not see KYUUBI_HOME env if we don't declare KYUUBI_HOME as an env var explicitly, this breaks the main resources jar discovery of the engine, and eventually fails the engine bootstrap. ## Describe Your Solution 🔧 Restore `export KYUUBI_HOME` logic in `load-kyuubi-env.sh` ## Types of changes 🔖 - [x] Bugfix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Test Plan 🧪 Build and start Kyuubi Server. ``` export SPARK_HOME=/Users/chengpan/app/spark-3.5.0-bin-hadoop3 build/dist --spark-provided --flink-provided --hive-provided dist/bin/kyuubi run ``` Open session to launch a Spark engine ``` dist/bin/beeline -u 'jdbc:hive2://0.0.0.0:10009/default' ``` #### Behavior Without This Pull Request ⚰️ A wrong `kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar` location was found. ``` /Users/chengpan/app/spark-3.5.0-bin-hadoop3/bin/spark-submit \ ... --proxy-user chengpan /Users/chengpan/Projects/apache-kyuubi/dist/jars/externals/kyuubi-spark-sql-engine/target/kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar ``` #### Behavior With This Pull Request 🎉 The correct `kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar` location was found. ``` /Users/chengpan/app/spark-3.5.0-bin-hadoop3/bin/spark-submit \ ... --proxy-user chengpan /Users/chengpan/Projects/apache-kyuubi/dist/externals/engines/spark/kyuubi-spark-sql-engine_2.12-1.9.0-SNAPSHOT.jar ``` --- # Checklist 📝 - [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html) **Be nice. Be informative.** Closes apache#6004 from pan3793/5987-followup. Closes apache#5987 a4e40b6 [Cheng Pan] [KYUUBI apache#5987] Always export KYUUBI_HOME in load-kyuubi-env.sh Authored-by: Cheng Pan <[email protected]> Signed-off-by: Cheng Pan <[email protected]>

[KYUUBI apache#5987] Always export KYUUBI_HOME in load-kyuubi-env.sh

a4e40b6

github-actions bot added the kind:deploy label Jan 22, 2024

pan3793 self-assigned this Jan 22, 2024

pan3793 added this to the v1.8.1 milestone Jan 22, 2024

pan3793 closed this in ac388d1 Jan 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KYUUBI #5987] Always export KYUUBI_HOME in `load-kyuubi-env.sh` #6004

[KYUUBI #5987] Always export KYUUBI_HOME in `load-kyuubi-env.sh` #6004

pan3793 commented Jan 22, 2024 •

edited

Loading

pan3793 commented Jan 22, 2024

codecov-commenter commented Jan 22, 2024

Emor-nj commented Jan 22, 2024 •

edited

Loading

pan3793 commented Jan 22, 2024

Emor-nj commented Jan 22, 2024

pan3793 commented Jan 22, 2024

[KYUUBI #5987] Always export KYUUBI_HOME in load-kyuubi-env.sh #6004

[KYUUBI #5987] Always export KYUUBI_HOME in load-kyuubi-env.sh #6004

Conversation

pan3793 commented Jan 22, 2024 • edited Loading

🔍 Description

Issue References 🔗

Describe Your Solution 🔧

Types of changes 🔖

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Checklist 📝

pan3793 commented Jan 22, 2024

codecov-commenter commented Jan 22, 2024

Codecov Report

Emor-nj commented Jan 22, 2024 • edited Loading

pan3793 commented Jan 22, 2024

Emor-nj commented Jan 22, 2024

pan3793 commented Jan 22, 2024

[KYUUBI #5987] Always export KYUUBI_HOME in `load-kyuubi-env.sh` #6004

[KYUUBI #5987] Always export KYUUBI_HOME in `load-kyuubi-env.sh` #6004

pan3793 commented Jan 22, 2024 •

edited

Loading

Emor-nj commented Jan 22, 2024 •

edited

Loading