Skip to content

Properly handle compute resources and number of executors #160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

sbernauer
Copy link
Member

@sbernauer sbernauer commented Oct 12, 2022

Description

Follow up of #147
Fixes the bug that passing the number of executors was accidentally removed.
It also fixes the unneeded pvc storage request.
It also adds the correct spark.driver.cores and spark.executor.cores so that the Pods use the available resources.

Review Checklist

  • Code contains useful comments
  • CRD change approved (or not applicable)
  • (Integration-)Test cases added (or not applicable)
  • Documentation added (or not applicable)
  • Changelog updated (or not applicable)
  • Cargo.toml only contains references to git tags (not specific commits or branches)
  • Helm chart can be installed and deployed operator works (or not applicable)

Once the review is done, comment bors r+ (or bors merge) to merge. Further information

@sbernauer sbernauer self-assigned this Oct 12, 2022
@sbernauer sbernauer marked this pull request as draft October 12, 2022 06:25
Comment on lines +949 to +950
// TODO Switch to assert!(command.contains(&"--conf spark.driver.cores=2".to_string()));
assert!(command.contains(&"--conf spark.driver.cores=150m".to_string()));
Copy link
Member Author

Choose a reason for hiding this comment

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

Letting the test pass so that i get a pr-version

Suggested change
// TODO Switch to assert!(command.contains(&"--conf spark.driver.cores=2".to_string()));
assert!(command.contains(&"--conf spark.driver.cores=150m".to_string()));
assert!(command.contains(&"--conf spark.driver.cores=2".to_string()));

@sbernauer sbernauer changed the title Properly handle compute resources Properly handle compute resources and number of executors Oct 12, 2022
@sbernauer
Copy link
Member Author

Closing this as it's more complicated. We have #161 to track the issue

@sbernauer sbernauer closed this Oct 12, 2022
@razvan razvan deleted the resource-handling branch January 20, 2023 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant