Skip to content

Conversation

@zhedoubushishi
Copy link
Contributor

Tips

What is the purpose of the pull request

When not passing SparkMaster, by default Hudi CLI should use SparkUtil.DEFAULT_SPARK_MASTER. However, w/ a recent code change in OSS, when SparkMaster is not passed, it would set Spark master to "" which causes the following exception when initializing a Hudi CLI job:

{{org.apache.spark.SparkException: Could not parse Master URL: ''at org.apache.spark.SparkContext$.org$apache$spark$SparkContext$$createTaskScheduler(SparkContext.scala:2999)

at org.apache.spark.SparkContext.<init>(SparkContext.scala:567)

at org.apache.spark.api.java.JavaSparkContext.<init>(JavaSparkContext.scala:58)

Brief change log

Verify this pull request

This change added tests and can be verified as follows:

  • Added testGetDefaultSparkConf to verify the change.

Committer checklist

  • Has a corresponding JIRA in PR title & commit

  • Commit message is descriptive of the change

  • CI is green

  • Necessary doc changes done or have another open PR

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

sparkMasterNode = properties.getProperty(HoodieCliSparkConfig.CLI_SPARK_MASTER);
}
sparkMasterNode = sparkMaster.orElse(sparkMasterNode);
if (sparkMaster.isPresent() && !sparkMaster.get().isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

!sparkMaster.get().isEmpty() -> !sparkMaster.get().trim().isEmpty()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

@nsivabalan nsivabalan assigned nsivabalan and unassigned nsivabalan Feb 22, 2022
@nsivabalan
Copy link
Contributor

@zhedoubushishi : can you address comments when you get a chance.
@XuQianJin-Stars : once you approve it, let me know. I can merge it in.

@zhedoubushishi
Copy link
Contributor Author

@nsivabalan Resolved the comment.

@hudi-bot
Copy link
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@XuQianJin-Stars
Copy link
Contributor

@zhedoubushishi : can you address comments when you get a chance. @XuQianJin-Stars : once you approve it, let me know. I can merge it in.

I approve it

@XuQianJin-Stars
Copy link
Contributor

LGTM , Thanks @zhedoubushishi

@nsivabalan nsivabalan added the priority:medium Moderate impact; usability gaps label Feb 28, 2022
@nsivabalan nsivabalan merged commit 18dc89c into apache:master Feb 28, 2022
rkkalluri pushed a commit to rkkalluri/hudi that referenced this pull request Mar 6, 2022
vingov pushed a commit to vingov/hudi that referenced this pull request Apr 3, 2022
stayrascal pushed a commit to stayrascal/hudi that referenced this pull request Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:medium Moderate impact; usability gaps

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants