You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "ksmou (via GitHub)" <gi...@apache.org> on 2023/06/12 15:55:09 UTC
[GitHub] [hudi] ksmou opened a new pull request, #8945: spark-memory parmater should not be forced to be true.
ksmou opened a new pull request, #8945:
URL: https://github.com/apache/hudi/pull/8945
### Change Logs
When using spark to submit compactor or clustering job, the spark-memory parameter is redundant. It should not be enforced to be true.
### Impact
none
### Risk level (write none, low medium or high below)
none
### Documentation Update
_Describe any necessary documentation update if there is any new feature, config, or user-facing change_
- _The config description must be updated if new configs are added or the default value of the configs are changed_
- _Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the [instruction](https://hudi.apache.org/contribute/developer-setup#website) to make
changes to the website._
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a diff in pull request #8945: [HUDI-6360] spark-memory paramater should not be required in compactor or clustering job
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8945:
URL: https://github.com/apache/hudi/pull/8945#discussion_r1230342721
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieClusteringJob.java:
##########
@@ -87,7 +87,7 @@ public static class Config implements Serializable {
public int parallelism = 1;
@Parameter(names = {"--spark-master", "-ms"}, description = "Spark master")
public String sparkMaster = null;
- @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = true)
+ @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = false)
public String sparkMemory = null;
Review Comment:
I'm wondering whether we should remove the option `--spark-memory` completely, like the usage of this options now is to override the memory value specified by `--executor-memory`, it has higher priority.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] ksmou commented on a diff in pull request #8945: [HUDI-6360]spark-memory parmater should not be forced to be true
Posted by "ksmou (via GitHub)" <gi...@apache.org>.
ksmou commented on code in PR #8945:
URL: https://github.com/apache/hudi/pull/8945#discussion_r1229591311
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieClusteringJob.java:
##########
@@ -87,7 +87,7 @@ public static class Config implements Serializable {
public int parallelism = 1;
@Parameter(names = {"--spark-master", "-ms"}, description = "Spark master")
public String sparkMaster = null;
- @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = true)
+ @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = false)
public String sparkMemory = null;
Review Comment:
In the defallt way, we must use `spark-submit --executor-memory=4g --spark-memory=4g`. However, we only use `spark-submit --executor-memory=4g` can meet it and `--spark-memory` should not keet it `required=true`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] ksmou commented on a diff in pull request #8945: [HUDI-6360]spark-memory parmater should not be forced to be true
Posted by "ksmou (via GitHub)" <gi...@apache.org>.
ksmou commented on code in PR #8945:
URL: https://github.com/apache/hudi/pull/8945#discussion_r1229273560
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieClusteringJob.java:
##########
@@ -87,7 +87,7 @@ public static class Config implements Serializable {
public int parallelism = 1;
@Parameter(names = {"--spark-master", "-ms"}, description = "Spark master")
public String sparkMaster = null;
- @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = true)
+ @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = false)
public String sparkMemory = null;
Review Comment:
The purpose of spark-memory is to set spark.executor.memory, which is usually configured according to executor-memory in Spark. We shouldd configure both executor-memory and spark-memory if spark-memory is forced to true.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a diff in pull request #8945: [HUDI-6360]spark-memory parmater should not be forced to be true
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8945:
URL: https://github.com/apache/hudi/pull/8945#discussion_r1229537703
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieClusteringJob.java:
##########
@@ -87,7 +87,7 @@ public static class Config implements Serializable {
public int parallelism = 1;
@Parameter(names = {"--spark-master", "-ms"}, description = "Spark master")
public String sparkMaster = null;
- @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = true)
+ @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = false)
public String sparkMemory = null;
Review Comment:
reasonable, then should we still keep this option then `--spark-memory` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a diff in pull request #8945: [HUDI-6360]spark-memory parmater should not be forced to be true
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8945:
URL: https://github.com/apache/hudi/pull/8945#discussion_r1228874997
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieClusteringJob.java:
##########
@@ -87,7 +87,7 @@ public static class Config implements Serializable {
public int parallelism = 1;
@Parameter(names = {"--spark-master", "-ms"}, description = "Spark master")
public String sparkMaster = null;
- @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = true)
+ @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = false)
public String sparkMemory = null;
Review Comment:
Not very familiar with Spark, what is the behavior when default memory is not suffient for job execution then?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 merged pull request #8945: [HUDI-6360] spark-memory paramater should not be required in compactor or clustering job
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 merged PR #8945:
URL: https://github.com/apache/hudi/pull/8945
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8945: [HUDI-6360]spark-memory parmater should not be forced to be true
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8945:
URL: https://github.com/apache/hudi/pull/8945#issuecomment-1588198127
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "09a49b9e90654c3b1ae1f32172af623b50011034",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17778",
"triggerID" : "09a49b9e90654c3b1ae1f32172af623b50011034",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 09a49b9e90654c3b1ae1f32172af623b50011034 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17778)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] ksmou commented on a diff in pull request #8945: [HUDI-6360] spark-memory paramater should not be required in compactor or clustering job
Posted by "ksmou (via GitHub)" <gi...@apache.org>.
ksmou commented on code in PR #8945:
URL: https://github.com/apache/hudi/pull/8945#discussion_r1230365325
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieClusteringJob.java:
##########
@@ -87,7 +87,7 @@ public static class Config implements Serializable {
public int parallelism = 1;
@Parameter(names = {"--spark-master", "-ms"}, description = "Spark master")
public String sparkMaster = null;
- @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = true)
+ @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = false)
public String sparkMemory = null;
Review Comment:
I think remove the option `--spark-memory` is also a good choice. Setting executor memroy completely depends on `--executor-memory`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a diff in pull request #8945: [HUDI-6360]spark-memory parmater should not be forced to be true
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8945:
URL: https://github.com/apache/hudi/pull/8945#discussion_r1229331929
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieClusteringJob.java:
##########
@@ -87,7 +87,7 @@ public static class Config implements Serializable {
public int parallelism = 1;
@Parameter(names = {"--spark-master", "-ms"}, description = "Spark master")
public String sparkMaster = null;
- @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = true)
+ @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = false)
public String sparkMemory = null;
Review Comment:
you mean we lack option for `executor-memory` ? How the user can config that with current maste code?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8945: [HUDI-6360]spark-memory parmater should not be forced to be true
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8945:
URL: https://github.com/apache/hudi/pull/8945#issuecomment-1587687423
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "09a49b9e90654c3b1ae1f32172af623b50011034",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "09a49b9e90654c3b1ae1f32172af623b50011034",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 09a49b9e90654c3b1ae1f32172af623b50011034 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] danny0405 commented on a diff in pull request #8945: [HUDI-6360] spark-memory paramater should not be required in compactor or clustering job
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #8945:
URL: https://github.com/apache/hudi/pull/8945#discussion_r1230367206
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieClusteringJob.java:
##########
@@ -87,7 +87,7 @@ public static class Config implements Serializable {
public int parallelism = 1;
@Parameter(names = {"--spark-master", "-ms"}, description = "Spark master")
public String sparkMaster = null;
- @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = true)
+ @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = false)
public String sparkMemory = null;
Review Comment:
If coexistence of different values from `--executor-memory` and `--spark-memory` is allowed, then keeping the options sounds good to me, it gives some space for flexibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8945: [HUDI-6360]spark-memory parmater should not be forced to be true
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8945:
URL: https://github.com/apache/hudi/pull/8945#issuecomment-1587698687
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "09a49b9e90654c3b1ae1f32172af623b50011034",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17778",
"triggerID" : "09a49b9e90654c3b1ae1f32172af623b50011034",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 09a49b9e90654c3b1ae1f32172af623b50011034 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17778)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] ksmou commented on a diff in pull request #8945: [HUDI-6360]spark-memory parmater should not be forced to be true
Posted by "ksmou (via GitHub)" <gi...@apache.org>.
ksmou commented on code in PR #8945:
URL: https://github.com/apache/hudi/pull/8945#discussion_r1229456639
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieClusteringJob.java:
##########
@@ -87,7 +87,7 @@ public static class Config implements Serializable {
public int parallelism = 1;
@Parameter(names = {"--spark-master", "-ms"}, description = "Spark master")
public String sparkMaster = null;
- @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = true)
+ @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = false)
public String sparkMemory = null;
Review Comment:
We use command 'spark-submit --executor-memory' can meet it, spark-memory is redundant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] ksmou commented on a diff in pull request #8945: [HUDI-6360]spark-memory parmater should not be forced to be true
Posted by "ksmou (via GitHub)" <gi...@apache.org>.
ksmou commented on code in PR #8945:
URL: https://github.com/apache/hudi/pull/8945#discussion_r1229591311
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieClusteringJob.java:
##########
@@ -87,7 +87,7 @@ public static class Config implements Serializable {
public int parallelism = 1;
@Parameter(names = {"--spark-master", "-ms"}, description = "Spark master")
public String sparkMaster = null;
- @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = true)
+ @Parameter(names = {"--spark-memory", "-sm"}, description = "spark memory to use", required = false)
public String sparkMemory = null;
Review Comment:
In the defallt way, we must use `spark-submit --executor-memory=4g --spark-memory=4g`. However, we only use `spark-submit --executor-memory=4g` can meet. It's no need to keep option `required=true` for `spark-memory`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org