You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "linliu-code (via GitHub)" <gi...@apache.org> on 2024/12/05 19:58:33 UTC
[PR] [HUDI-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
linliu-code opened a new pull request, #12433:
URL: https://github.com/apache/hudi/pull/12433
### Change Logs
Clean the logic of this merger, since it is event time based.
### Impact
Event time based handling for delete records.
### Risk level (write none, low medium or high below)
Low.
### Documentation Update
_Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none"._
- _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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on code in PR #12433:
URL: https://github.com/apache/hudi/pull/12433#discussion_r1872322604
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/DefaultSparkRecordMerger.java:
##########
@@ -48,31 +49,6 @@ public Option<Pair<HoodieRecord, Schema>> merge(HoodieRecord older, Schema oldSc
ValidationUtils.checkArgument(older.getRecordType() == HoodieRecordType.SPARK);
ValidationUtils.checkArgument(newer.getRecordType() == HoodieRecordType.SPARK);
- if (newer instanceof HoodieSparkRecord) {
Review Comment:
Do you mean we will remove this merger classes in 1.0.1? Sounds like big change.
--
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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code closed pull request #12433: [HUDI-7612] Use event time ordering for DefaultSparkRecordMerger
URL: https://github.com/apache/hudi/pull/12433
--
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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on code in PR #12433:
URL: https://github.com/apache/hudi/pull/12433#discussion_r1872324005
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/DefaultSparkRecordMerger.java:
##########
@@ -48,31 +49,6 @@ public Option<Pair<HoodieRecord, Schema>> merge(HoodieRecord older, Schema oldSc
ValidationUtils.checkArgument(older.getRecordType() == HoodieRecordType.SPARK);
ValidationUtils.checkArgument(newer.getRecordType() == HoodieRecordType.SPARK);
- if (newer instanceof HoodieSparkRecord) {
Review Comment:
Ok, let us make sure to talk about this in today's sync up.
--
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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on PR #12433:
URL: https://github.com/apache/hudi/pull/12433#issuecomment-2524726348
I will abandon this change, and prepare a new one.
--
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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12433:
URL: https://github.com/apache/hudi/pull/12433#issuecomment-2521436382
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d905b7d729d1183ff13597875016883c729a9564",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2308",
"triggerID" : "d905b7d729d1183ff13597875016883c729a9564",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d905b7d729d1183ff13597875016883c729a9564 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2308)
<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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #12433:
URL: https://github.com/apache/hudi/pull/12433#discussion_r1872167617
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/DefaultSparkRecordMerger.java:
##########
@@ -48,31 +49,6 @@ public Option<Pair<HoodieRecord, Schema>> merge(HoodieRecord older, Schema oldSc
ValidationUtils.checkArgument(older.getRecordType() == HoodieRecordType.SPARK);
ValidationUtils.checkArgument(newer.getRecordType() == HoodieRecordType.SPARK);
- if (newer instanceof HoodieSparkRecord) {
Review Comment:
If this class is no longer used in actual read and write path, I'm not sure if there's value fixing it.
--
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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #12433:
URL: https://github.com/apache/hudi/pull/12433#discussion_r1872020410
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/DefaultSparkRecordMerger.java:
##########
@@ -48,31 +49,6 @@ public Option<Pair<HoodieRecord, Schema>> merge(HoodieRecord older, Schema oldSc
ValidationUtils.checkArgument(older.getRecordType() == HoodieRecordType.SPARK);
ValidationUtils.checkArgument(newer.getRecordType() == HoodieRecordType.SPARK);
- if (newer instanceof HoodieSparkRecord) {
Review Comment:
We should think about deleting the merge classes for `COMMIT_TIME_ORDERING` AND `EVENT_TIME_ORDERING` because they are directly implemented in the file group reader record buffer.
--
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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12433:
URL: https://github.com/apache/hudi/pull/12433#issuecomment-2521279080
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d905b7d729d1183ff13597875016883c729a9564",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "d905b7d729d1183ff13597875016883c729a9564",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d905b7d729d1183ff13597875016883c729a9564 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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on code in PR #12433:
URL: https://github.com/apache/hudi/pull/12433#discussion_r1872039956
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/DefaultSparkRecordMerger.java:
##########
@@ -48,31 +49,6 @@ public Option<Pair<HoodieRecord, Schema>> merge(HoodieRecord older, Schema oldSc
ValidationUtils.checkArgument(older.getRecordType() == HoodieRecordType.SPARK);
ValidationUtils.checkArgument(newer.getRecordType() == HoodieRecordType.SPARK);
- if (newer instanceof HoodieSparkRecord) {
Review Comment:
How about these merge logic in mergeHandles? Are we going to remove merger from them? Or replace them with fg reader?
--
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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on code in PR #12433:
URL: https://github.com/apache/hudi/pull/12433#discussion_r1872039956
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/DefaultSparkRecordMerger.java:
##########
@@ -48,31 +49,6 @@ public Option<Pair<HoodieRecord, Schema>> merge(HoodieRecord older, Schema oldSc
ValidationUtils.checkArgument(older.getRecordType() == HoodieRecordType.SPARK);
ValidationUtils.checkArgument(newer.getRecordType() == HoodieRecordType.SPARK);
- if (newer instanceof HoodieSparkRecord) {
Review Comment:
How about these merge logic in mergeHandles? Are we going to remove merger from them?
--
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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on code in PR #12433:
URL: https://github.com/apache/hudi/pull/12433#discussion_r1872085476
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/DefaultSparkRecordMerger.java:
##########
@@ -48,31 +49,6 @@ public Option<Pair<HoodieRecord, Schema>> merge(HoodieRecord older, Schema oldSc
ValidationUtils.checkArgument(older.getRecordType() == HoodieRecordType.SPARK);
ValidationUtils.checkArgument(newer.getRecordType() == HoodieRecordType.SPARK);
- if (newer instanceof HoodieSparkRecord) {
Review Comment:
@yihua , I think what you mentioned above is orthogonal to this ticket. We could think through the big picture, and come up with a long term design/solution; but that should be out of scope of this ticket.
--
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-7612] Use event time ordering for DefaultSparkRecordMerger [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #12433:
URL: https://github.com/apache/hudi/pull/12433#issuecomment-2521281634
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d905b7d729d1183ff13597875016883c729a9564",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2308",
"triggerID" : "d905b7d729d1183ff13597875016883c729a9564",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d905b7d729d1183ff13597875016883c729a9564 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=2308)
<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