[SPARK-32227] Fix regression bug in load-spark-env.cmd with Spark 3.0.0 #29044

warrenzhu25 · 2020-07-08T16:18:29Z

What changes were proposed in this pull request?

Fix regression bug in load-spark-env.cmd with Spark 3.0.0

Why are the changes needed?

cmd doesn't support set env twice. So set SPARK_ENV_CMD=%SPARK_CONF_DIR%\%SPARK_ENV_CMD% doesn't take effect, which caused regression.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Manually tested.

Create a spark-env.cmd under conf folder. Inside this, echo spark-env.cmd
Run old load-spark-env.cmd, nothing printed in the output
Run fixed load-spark-env.cmd, spark-env.cmd showed in the output.

bin/load-spark-env.cmd

dongjoon-hyun · 2020-07-08T16:25:12Z

cc @srowen

dongjoon-hyun · 2020-07-08T16:26:06Z

@warrenzhu25 . Could you elaborate about what is the regression you are mentioning here?

Fix regression bug in load-spark-env.cmd with Spark 3.0.0

HyukjinKwon · 2020-07-09T07:06:19Z

ok to test

bin/load-spark-env.cmd

HyukjinKwon · 2020-07-09T07:12:53Z

@warrenzhu25 it should be best to show the input like your command you tried and output from the command explicitly before/after the fix in PR description especially given that not so many dev people here arguably don't have Windows env.

SparkQA · 2020-07-09T10:42:17Z

Test build #125441 has finished for PR 29044 at commit bb018bb.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2020-07-09T19:03:38Z

@warrenzhu25 As you see, we have a Windows OS test job in AppVeyor.
This patch consistently fails there. Could you take a look at that?

https://ci.appveyor.com/project/ApacheSoftwareFoundation/spark/builds/33982534

dongjoon-hyun

Currently, master branch is healthy on Windows.

https://ci.appveyor.com/project/ApacheSoftwareFoundation/spark/builds/33996238

To be considered as a valid patch, this PR should pass AppVeyor test job on Windows.

warrenzhu25 · 2020-07-10T15:17:18Z

@warrenzhu25 As you see, we have a Windows OS test job in AppVeyor.
This patch consistently fails there. Could you take a look at that?

https://ci.appveyor.com/project/ApacheSoftwareFoundation/spark/builds/33982534

As I see, the failure seems unrelated with this change. It seems it couldn't find correct version of hadoop.dll.

dongjoon-hyun · 2020-07-10T16:11:55Z

It's directly relevant to this PR because your patch is changing environment variable.

Please see this for the detail (https://github.com/cdarlint/winutils)
You can run AppVeyor in your Spark fork, too.

dongjoon-hyun · 2020-07-10T16:12:38Z

Please remove [WIP] from the title when AppVeyor passes on Windows. Thanks.

warrenzhu25 · 2020-07-10T16:39:15Z

It's directly relevant to this PR because your patch is changing environment variable.

Please see this for the detail (https://github.com/cdarlint/winutils)

You can run AppVeyor in your Spark fork, too.

winutils only impacted by PATH and HADOOP_HOME, and I don't touch both. Also, my change is just reverting into the version as 2.4.4. Could you help rerun the tests?

warrenzhu25 · 2020-07-21T16:41:39Z

@dongjoon-hyun Could you help retest this as failing tests might be unrelated?

HyukjinKwon · 2020-07-27T04:00:31Z

@warrenzhu25 you can push empty commit to retrigger the tests, or rebase to sync with the master branch.

dongjoon-hyun · 2020-07-27T04:22:50Z

+1 for @HyukjinKwon 's comment.

HyukjinKwon · 2020-07-27T06:27:53Z

Okay, I am debugging a related issue and happened to take a look again for this fix. this fix looks good. AppVeyor currently fails for another issue which I'll probably be able to fix soon.

SparkQA · 2020-07-27T07:05:02Z

Test build #126617 has finished for PR 29044 at commit d6219d8.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-07-27T20:37:46Z

Test build #126659 has finished for PR 29044 at commit d169de3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-07-28T00:04:55Z

Test build #126670 has finished for PR 29044 at commit 24af13c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-07-28T05:02:08Z

Test build #126680 has finished for PR 29044 at commit 4589e1b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

warrenzhu25 · 2020-07-29T03:57:58Z

@dongjoon-hyun Could you help merge this?

HyukjinKwon · 2020-07-30T12:44:36Z

Merged to master and branch-3.0.

HyukjinKwon · 2020-07-30T12:45:00Z

Thanks for working on this @warrenzhu25.

Fix regression bug in load-spark-env.cmd with Spark 3.0.0 cmd doesn't support set env twice. So set `SPARK_ENV_CMD=%SPARK_CONF_DIR%\%SPARK_ENV_CMD%` doesn't take effect, which caused regression. No Manually tested. 1. Create a spark-env.cmd under conf folder. Inside this, `echo spark-env.cmd` 2. Run old load-spark-env.cmd, nothing printed in the output 2. Run fixed load-spark-env.cmd, `spark-env.cmd` showed in the output. Closes #29044 from warrenzhu25/32227. Lead-authored-by: Warren Zhu <[email protected]> Co-authored-by: Warren Zhu <[email protected]> Signed-off-by: HyukjinKwon <[email protected]> (cherry picked from commit 7437720) Signed-off-by: HyukjinKwon <[email protected]>

dongjoon-hyun · 2020-07-30T14:48:31Z

Thank you all. I see that this passed the GitHub Action at 4589e1b .

probot-autolabeler bot added the WINDOWS label Jul 8, 2020

dongjoon-hyun reviewed Jul 8, 2020

View reviewed changes

bin/load-spark-env.cmd Show resolved Hide resolved

HyukjinKwon reviewed Jul 9, 2020

View reviewed changes

bin/load-spark-env.cmd Show resolved Hide resolved

dongjoon-hyun requested changes Jul 9, 2020

View reviewed changes

dongjoon-hyun changed the title ~~[SPARK-32227] Fix regression bug in load-spark-env.cmd with Spark 3.0.0~~ [WIP][SPARK-32227] Fix regression bug in load-spark-env.cmd with Spark 3.0.0 Jul 10, 2020

warrenzhu25 changed the title ~~[WIP][SPARK-32227] Fix regression bug in load-spark-env.cmd with Spark 3.0.0~~ [SPARK-32227] Fix regression bug in load-spark-env.cmd with Spark 3.0.0 Jul 27, 2020

warrenzhu25 force-pushed the 32227 branch from 24af13c to f5292dc Compare July 28, 2020 00:56

Warren Zhu and others added 2 commits July 27, 2020 17:57

[SPARK-32227] Fix regression bug in load-spark-env.cmd with Spark 3.0.0

1d98751

Trigger Build

4589e1b

warrenzhu25 force-pushed the 32227 branch from f5292dc to 4589e1b Compare July 28, 2020 00:59

HyukjinKwon approved these changes Jul 28, 2020

View reviewed changes

HyukjinKwon closed this in 7437720 Jul 30, 2020

warrenzhu25 deleted the 32227 branch June 11, 2022 17:14

[SPARK-32227] Fix regression bug in load-spark-env.cmd with Spark 3.0.0 #29044

[SPARK-32227] Fix regression bug in load-spark-env.cmd with Spark 3.0.0 #29044

Uh oh!

Conversation

warrenzhu25 commented Jul 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Uh oh!

dongjoon-hyun commented Jul 8, 2020

Uh oh!

dongjoon-hyun commented Jul 8, 2020

Uh oh!

HyukjinKwon commented Jul 9, 2020

Uh oh!

Uh oh!

HyukjinKwon commented Jul 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Jul 9, 2020

Uh oh!

dongjoon-hyun commented Jul 9, 2020

Uh oh!

dongjoon-hyun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

warrenzhu25 commented Jul 10, 2020

Uh oh!

dongjoon-hyun commented Jul 10, 2020

Uh oh!

dongjoon-hyun commented Jul 10, 2020

Uh oh!

warrenzhu25 commented Jul 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

warrenzhu25 commented Jul 21, 2020

Uh oh!

HyukjinKwon commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun commented Jul 27, 2020

Uh oh!

HyukjinKwon commented Jul 27, 2020

Uh oh!

SparkQA commented Jul 27, 2020

Uh oh!

SparkQA commented Jul 27, 2020

Uh oh!

SparkQA commented Jul 28, 2020

Uh oh!

SparkQA commented Jul 28, 2020

Uh oh!

warrenzhu25 commented Jul 29, 2020

Uh oh!

HyukjinKwon commented Jul 30, 2020

Uh oh!

HyukjinKwon commented Jul 30, 2020

Uh oh!

dongjoon-hyun commented Jul 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

warrenzhu25 commented Jul 8, 2020 •

edited

Loading

HyukjinKwon commented Jul 9, 2020 •

edited

Loading

dongjoon-hyun left a comment •

edited

Loading

warrenzhu25 commented Jul 10, 2020 •

edited

Loading

HyukjinKwon commented Jul 27, 2020 •

edited

Loading