You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "wombatu-kun (via GitHub)" <gi...@apache.org> on 2024/02/20 10:47:02 UTC
[PR] [HUDI-4444] Refactor DataSourceInternalWriterHelper [hudi]
wombatu-kun opened a new pull request, #10715:
URL: https://github.com/apache/hudi/pull/10715
### Change Logs
DataSourceInternalWriterHelper constructor is writing files (through writeClient.startCommitWithTime and writeClient.preWrite), which is an anti-pattern.
Moved code that starts transaction (creates files through startCommitWithTime and preWrite) from constructor to separate method startTransaction() and called it from createInflightCommit().
### 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
Re: [PR] [HUDI-4444] Refactor DataSourceInternalWriterHelper [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10715:
URL: https://github.com/apache/hudi/pull/10715#discussion_r1498551962
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/internal/DataSourceInternalWriterHelper.java:
##########
@@ -66,13 +66,11 @@ public DataSourceInternalWriterHelper(String instantTime, HoodieWriteConfig writ
this.extraMetadata = extraMetadata;
this.writeClient = new SparkRDDWriteClient<>(new HoodieSparkEngineContext(new JavaSparkContext(sparkSession.sparkContext())), writeConfig);
this.writeClient.setOperationType(operationType);
- this.writeClient.startCommitWithTime(instantTime);
this.writeClient.initTable(operationType, Option.of(instantTime));
Review Comment:
Not sure if `initTable` requires the requested commit file to be existed.
--
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
Re: [PR] [HUDI-4444] Refactor DataSourceInternalWriterHelper [hudi]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun closed pull request #10715: [HUDI-4444] Refactor DataSourceInternalWriterHelper
URL: https://github.com/apache/hudi/pull/10715
--
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
Re: [PR] [HUDI-4444] Refactor DataSourceInternalWriterHelper [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10715:
URL: https://github.com/apache/hudi/pull/10715#issuecomment-1954014965
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "e2c574be3c2f8953e3973610fb0e20fef16f68ba",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22523",
"triggerID" : "e2c574be3c2f8953e3973610fb0e20fef16f68ba",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* e2c574be3c2f8953e3973610fb0e20fef16f68ba Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22523)
<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
Re: [PR] [HUDI-4444] Refactor DataSourceInternalWriterHelper [hudi]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on code in PR #10715:
URL: https://github.com/apache/hudi/pull/10715#discussion_r1501346077
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/internal/DataSourceInternalWriterHelper.java:
##########
@@ -66,13 +66,11 @@ public DataSourceInternalWriterHelper(String instantTime, HoodieWriteConfig writ
this.extraMetadata = extraMetadata;
this.writeClient = new SparkRDDWriteClient<>(new HoodieSparkEngineContext(new JavaSparkContext(sparkSession.sparkContext())), writeConfig);
this.writeClient.setOperationType(operationType);
- this.writeClient.startCommitWithTime(instantTime);
this.writeClient.initTable(operationType, Option.of(instantTime));
Review Comment:
oh, yes, may be you are right, but all tests pass succesfully. Can you give me some advice to make this PR correct?
--
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
Re: [PR] [HUDI-4444] Refactor DataSourceInternalWriterHelper [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10715:
URL: https://github.com/apache/hudi/pull/10715#discussion_r1501963827
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/internal/DataSourceInternalWriterHelper.java:
##########
@@ -66,13 +66,11 @@ public DataSourceInternalWriterHelper(String instantTime, HoodieWriteConfig writ
this.extraMetadata = extraMetadata;
this.writeClient = new SparkRDDWriteClient<>(new HoodieSparkEngineContext(new JavaSparkContext(sparkSession.sparkContext())), writeConfig);
this.writeClient.setOperationType(operationType);
- this.writeClient.startCommitWithTime(instantTime);
this.writeClient.initTable(operationType, Option.of(instantTime));
Review Comment:
I have no strong advices in this moment, the lifecycle methods in write client has it's own dependencies, this is really a pain to maintainence, maybe we should add more docs for them first and find out a way to make the method invocation more safe.
--
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
Re: [PR] [HUDI-4444] Refactor DataSourceInternalWriterHelper [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10715:
URL: https://github.com/apache/hudi/pull/10715#issuecomment-1954217729
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "e2c574be3c2f8953e3973610fb0e20fef16f68ba",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22523",
"triggerID" : "e2c574be3c2f8953e3973610fb0e20fef16f68ba",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* e2c574be3c2f8953e3973610fb0e20fef16f68ba Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22523)
<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
Re: [PR] [HUDI-4444] Refactor DataSourceInternalWriterHelper [hudi]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on code in PR #10715:
URL: https://github.com/apache/hudi/pull/10715#discussion_r1500706025
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/internal/DataSourceInternalWriterHelper.java:
##########
@@ -66,13 +66,11 @@ public DataSourceInternalWriterHelper(String instantTime, HoodieWriteConfig writ
this.extraMetadata = extraMetadata;
this.writeClient = new SparkRDDWriteClient<>(new HoodieSparkEngineContext(new JavaSparkContext(sparkSession.sparkContext())), writeConfig);
this.writeClient.setOperationType(operationType);
- this.writeClient.startCommitWithTime(instantTime);
this.writeClient.initTable(operationType, Option.of(instantTime));
Review Comment:
Definitely, initTable does not require the requested commit file to be existed. That's why i moved invocation of startCommitWithTime out of constructor right to that place, where it is needed (first statement of createInflightCommit method).
--
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
Re: [PR] [HUDI-4444] Refactor DataSourceInternalWriterHelper [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10715:
URL: https://github.com/apache/hudi/pull/10715#issuecomment-1954000931
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "e2c574be3c2f8953e3973610fb0e20fef16f68ba",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "e2c574be3c2f8953e3973610fb0e20fef16f68ba",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* e2c574be3c2f8953e3973610fb0e20fef16f68ba 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
Re: [PR] [HUDI-4444] Refactor DataSourceInternalWriterHelper [hudi]
Posted by "danny0405 (via GitHub)" <gi...@apache.org>.
danny0405 commented on code in PR #10715:
URL: https://github.com/apache/hudi/pull/10715#discussion_r1501342066
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/internal/DataSourceInternalWriterHelper.java:
##########
@@ -66,13 +66,11 @@ public DataSourceInternalWriterHelper(String instantTime, HoodieWriteConfig writ
this.extraMetadata = extraMetadata;
this.writeClient = new SparkRDDWriteClient<>(new HoodieSparkEngineContext(new JavaSparkContext(sparkSession.sparkContext())), writeConfig);
this.writeClient.setOperationType(operationType);
- this.writeClient.startCommitWithTime(instantTime);
this.writeClient.initTable(operationType, Option.of(instantTime));
Review Comment:
The `initTable` has a param `Option<String> instantTime` which is used for startting the trasanction conflict process, not sure what happens if the metadata file does not exist there at all.
--
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