You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2022/11/17 06:50:02 UTC
[GitHub] [hudi] xicm opened a new pull request, #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
xicm opened a new pull request, #7226:
URL: https://github.com/apache/hudi/pull/7226
### Change Logs
Make user-provided COPY_ON_WRITE_RECORD_SIZE_ESTIMATE first
1) if user sets a value, then use it as is
2) if user not setting it, infer from timeline commit metadata
3) if timeline is empty, use a default (current: 1024)
### Impact
HoodieCompactionConfig.COPY_ON_WRITE_RECORD_SIZE_ESTIMATE
### Risk level (write none, low medium or high below)
low
_If medium or high, explain what verification was done to mitigate the risks._
### 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] hudi-bot commented on pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7226:
URL: https://github.com/apache/hudi/pull/7226#issuecomment-1318193737
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13071",
"triggerID" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 89e408f8aa68139a5f87230f1d3f4f3a5fb838d8 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13071)
<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] hudi-bot commented on pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7226:
URL: https://github.com/apache/hudi/pull/7226#issuecomment-1321450889
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13071",
"triggerID" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"triggerType" : "PUSH"
}, {
"hash" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13137",
"triggerID" : "1321341828",
"triggerType" : "MANUAL"
} ]
}-->
## CI report:
* 89e408f8aa68139a5f87230f1d3f4f3a5fb838d8 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13071) Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13137)
<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 #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
danny0405 commented on code in PR #7226:
URL: https://github.com/apache/hudi/pull/7226#discussion_r1027670000
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -367,9 +368,19 @@ public int getPartition(Object key) {
/**
* Obtains the average record size based on records written during previous commits. Used for estimating how many
* records pack into one file.
+ * Respect user setting by following the precedence as below
+ * 1) if user sets a value, then use it as is
+ * 2) if user not setting it, infer from timeline commit metadata
+ * 3) if timeline is empty, use a default (current: 1024)
*/
protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, HoodieWriteConfig hoodieWriteConfig) {
+ long defaultAvgSize = Integer.parseInt(HoodieCompactionConfig.COPY_ON_WRITE_RECORD_SIZE_ESTIMATE.defaultValue());
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
+
+ if (avgSize != defaultAvgSize) {
Review Comment:
Yeah, you need to explain why the explicit record size is always considered while ignoring the write stats from the commit metadata ? Can we elaborate to make the commit metadata more accurate here ?
--
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] xicm closed pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by "xicm (via GitHub)" <gi...@apache.org>.
xicm closed pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
URL: https://github.com/apache/hudi/pull/7226
--
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 #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7226:
URL: https://github.com/apache/hudi/pull/7226#issuecomment-1318188987
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 89e408f8aa68139a5f87230f1d3f4f3a5fb838d8 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] hudi-bot commented on pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7226:
URL: https://github.com/apache/hudi/pull/7226#issuecomment-1321342297
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13071",
"triggerID" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"triggerType" : "PUSH"
}, {
"hash" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13137",
"triggerID" : "1321341828",
"triggerType" : "MANUAL"
} ]
}-->
## CI report:
* 89e408f8aa68139a5f87230f1d3f4f3a5fb838d8 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13071) Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13137)
<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] hudi-bot commented on pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7226:
URL: https://github.com/apache/hudi/pull/7226#issuecomment-1318366475
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13071",
"triggerID" : "89e408f8aa68139a5f87230f1d3f4f3a5fb838d8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 89e408f8aa68139a5f87230f1d3f4f3a5fb838d8 Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13071)
<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] xicm commented on a diff in pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
xicm commented on code in PR #7226:
URL: https://github.com/apache/hudi/pull/7226#discussion_r1028925825
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -367,9 +368,19 @@ public int getPartition(Object key) {
/**
* Obtains the average record size based on records written during previous commits. Used for estimating how many
* records pack into one file.
+ * Respect user setting by following the precedence as below
+ * 1) if user sets a value, then use it as is
+ * 2) if user not setting it, infer from timeline commit metadata
+ * 3) if timeline is empty, use a default (current: 1024)
*/
protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, HoodieWriteConfig hoodieWriteConfig) {
+ long defaultAvgSize = Integer.parseInt(HoodieCompactionConfig.COPY_ON_WRITE_RECORD_SIZE_ESTIMATE.defaultValue());
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
+
+ if (avgSize != defaultAvgSize) {
Review Comment:
Hi @danny0405 , do you mean to add some comment to explain why we ignore the write stats from the commit metadata and how to set the value more accurate?
--
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 #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
danny0405 commented on code in PR #7226:
URL: https://github.com/apache/hudi/pull/7226#discussion_r1028937411
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -367,9 +368,19 @@ public int getPartition(Object key) {
/**
* Obtains the average record size based on records written during previous commits. Used for estimating how many
* records pack into one file.
+ * Respect user setting by following the precedence as below
+ * 1) if user sets a value, then use it as is
+ * 2) if user not setting it, infer from timeline commit metadata
+ * 3) if timeline is empty, use a default (current: 1024)
*/
protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, HoodieWriteConfig hoodieWriteConfig) {
+ long defaultAvgSize = Integer.parseInt(HoodieCompactionConfig.COPY_ON_WRITE_RECORD_SIZE_ESTIMATE.defaultValue());
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
+
+ if (avgSize != defaultAvgSize) {
Review Comment:
I mean we should figure out why the write stats is not that accurate ?
--
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] xicm commented on pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
xicm commented on PR #7226:
URL: https://github.com/apache/hudi/pull/7226#issuecomment-1321341828
@hudi-bot run azure
--
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] xicm commented on a diff in pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
xicm commented on code in PR #7226:
URL: https://github.com/apache/hudi/pull/7226#discussion_r1029073719
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -367,9 +368,19 @@ public int getPartition(Object key) {
/**
* Obtains the average record size based on records written during previous commits. Used for estimating how many
* records pack into one file.
+ * Respect user setting by following the precedence as below
+ * 1) if user sets a value, then use it as is
+ * 2) if user not setting it, infer from timeline commit metadata
+ * 3) if timeline is empty, use a default (current: 1024)
*/
protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, HoodieWriteConfig hoodieWriteConfig) {
+ long defaultAvgSize = Integer.parseInt(HoodieCompactionConfig.COPY_ON_WRITE_RECORD_SIZE_ESTIMATE.defaultValue());
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
+
+ if (avgSize != defaultAvgSize) {
Review Comment:
@danny0405 The calculation of average value is OK from my side.
@xushiyan could you explain the purpose of making user-provided first？Are these changes what you expected？
--
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] xicm commented on pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
xicm commented on PR #7226:
URL: https://github.com/apache/hudi/pull/7226#issuecomment-1321470483
> Considering this scenario, I will set `COPY_ON_WRITE_RECORD_SIZE_ESTIMATE` is more smaller than the original to prevent generate a large number of small files when I first load data. But I can't accurately estimate the size of each record after load in hudi, only use the original data meta. And next time, if I hava history data in hudi table, I tend to use the existing data to calculate the record size. And if the schema is evolution, the user provide may be will inaccurate, they need to reset the conf, right?
@KnightChess Yes, you are right.
The original logic has a problem that user provided avg value does not take effect.
@xushiyan could you explain more about the original purpose of this issue？
--
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 #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
danny0405 commented on code in PR #7226:
URL: https://github.com/apache/hudi/pull/7226#discussion_r1027509172
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -367,9 +368,19 @@ public int getPartition(Object key) {
/**
* Obtains the average record size based on records written during previous commits. Used for estimating how many
* records pack into one file.
+ * Respect user setting by following the precedence as below
+ * 1) if user sets a value, then use it as is
+ * 2) if user not setting it, infer from timeline commit metadata
+ * 3) if timeline is empty, use a default (current: 1024)
*/
protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, HoodieWriteConfig hoodieWriteConfig) {
+ long defaultAvgSize = Integer.parseInt(HoodieCompactionConfig.COPY_ON_WRITE_RECORD_SIZE_ESTIMATE.defaultValue());
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
+
+ if (avgSize != defaultAvgSize) {
Review Comment:
The `HoodieCompactionConfig#build` already sets the defaults explicitly if it is not configured, so what's the problem here ?
--
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 #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
danny0405 commented on code in PR #7226:
URL: https://github.com/apache/hudi/pull/7226#discussion_r1027510067
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -367,9 +368,19 @@ public int getPartition(Object key) {
/**
* Obtains the average record size based on records written during previous commits. Used for estimating how many
* records pack into one file.
+ * Respect user setting by following the precedence as below
+ * 1) if user sets a value, then use it as is
+ * 2) if user not setting it, infer from timeline commit metadata
+ * 3) if timeline is empty, use a default (current: 1024)
*/
protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, HoodieWriteConfig hoodieWriteConfig) {
+ long defaultAvgSize = Integer.parseInt(HoodieCompactionConfig.COPY_ON_WRITE_RECORD_SIZE_ESTIMATE.defaultValue());
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
+
+ if (avgSize != defaultAvgSize) {
Review Comment:
I see, you didn't what the moving average from the commit matadata, is the moving average not accurate or something else ?
--
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] xicm commented on a diff in pull request #7226: [HUDI-5018] Make user-provided copyOnWriteRecordSizeEstimate first precedence
Posted by GitBox <gi...@apache.org>.
xicm commented on code in PR #7226:
URL: https://github.com/apache/hudi/pull/7226#discussion_r1027564734
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -367,9 +368,19 @@ public int getPartition(Object key) {
/**
* Obtains the average record size based on records written during previous commits. Used for estimating how many
* records pack into one file.
+ * Respect user setting by following the precedence as below
+ * 1) if user sets a value, then use it as is
+ * 2) if user not setting it, infer from timeline commit metadata
+ * 3) if timeline is empty, use a default (current: 1024)
*/
protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, HoodieWriteConfig hoodieWriteConfig) {
+ long defaultAvgSize = Integer.parseInt(HoodieCompactionConfig.COPY_ON_WRITE_RECORD_SIZE_ESTIMATE.defaultValue());
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
+
+ if (avgSize != defaultAvgSize) {
Review Comment:
@danny0405 This pr makes user-provided avg value first. We can't tell the value is default or user-provided here, so I suppose if the avg size is not default, it is user provided.
--
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