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/21 03:06:45 UTC
[GitHub] [hudi] honeyaya opened a new pull request, #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
honeyaya opened a new pull request, #7255:
URL: https://github.com/apache/hudi/pull/7255
Using the default value of estimate record size at the averageBytesPerRecord() when estimation threshold is less than 0
### Change Logs
Currently, hudi obtains the average record size based on records written during previous commits. Used for estimating how many records pack into one file, and the code is about UpsertPartitioner.averageBytesPerRecord().
But we found that the single data file could become 600~700M and most other files are less than 200M.
- Reason
1. the result of totalBytesWritten/totalRecordsWritten is very small when the last commit, but the next commit record is very large, then the data files will become very large.
- Solve plan
1. Plan1: calculate avgSize of the past several commit not just only one, but the getCommitMetadata costs a lot of time, then this function might be slow, so we did not choose this.
1. Plan2: Use the estimated record size considering our data size is fixed in some sense, more the hudi community did not encourage adding a more boolean variable to control whether to use the last commit avgSize, then we use the estimation threshold, when it is less than 0, we use the default estimate record size.
### Impact
UpsertPartitioner.averageBytesPerRecord(), small
### Risk level (write none, low medium or high below)
low, this feature works only when estimation threshold is less than 0
### Documentation Update
_Describe any necessary documentation update if there is any new feature, config, or user-facing change_
- _The config description must be updated_
> We use the previous commits' metadata to calculate the estimated record size and use it "
+ " to bin pack records into partitions. If the previous commit is too small to make an accurate estimation, "
+ " Hudi will search commits in the reverse order, until we find a commit that has totalBytesWritten "
+ " larger than (PARQUET_SMALL_FILE_LIMIT_BYTES * this_threshold). Will use hoodie.copyonwrite.record.size.estimate value when this value is less than 0.");
### 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 #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7255:
URL: https://github.com/apache/hudi/pull/7255#issuecomment-1321397575
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "98941b2fe0f47e0d3d0846cc7869c29eed4d090d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "98941b2fe0f47e0d3d0846cc7869c29eed4d090d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 98941b2fe0f47e0d3d0846cc7869c29eed4d090d 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 #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on code in PR #7255:
URL: https://github.com/apache/hudi/pull/7255#discussion_r1027545946
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -372,7 +372,7 @@ protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, Hoodi
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
long fileSizeThreshold = (long) (hoodieWriteConfig.getRecordSizeEstimationThreshold() * hoodieWriteConfig.getParquetSmallFileLimit());
try {
- if (!commitTimeline.empty()) {
+ if (hoodieWriteConfig.getRecordSizeEstimationThreshold() > 0 && !commitTimeline.empty()) {
// Go over the reverse ordered commits to get a more recent estimate of average record size.
Iterator<HoodieInstant> instants = commitTimeline.getReverseOrderedInstants().iterator();
Review Comment:
> data size is fixed in some sense, more the hudi community did not encourage adding a more boolean variable to control whether to use the last commit avgSize, then we use the estimation threshold, when it is less than 0, we use the default estimate record size.
Did you mean the records size Variance is large ？
--
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] helpta commented on pull request #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by "helpta (via GitHub)" <gi...@apache.org>.
helpta commented on PR #7255:
URL: https://github.com/apache/hudi/pull/7255#issuecomment-1449288020
Will this change be applied to other clients?
--
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] helpta commented on a diff in pull request #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by "helpta (via GitHub)" <gi...@apache.org>.
helpta commented on code in PR #7255:
URL: https://github.com/apache/hudi/pull/7255#discussion_r1111450040
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -372,7 +372,7 @@ protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, Hoodi
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
long fileSizeThreshold = (long) (hoodieWriteConfig.getRecordSizeEstimationThreshold() * hoodieWriteConfig.getParquetSmallFileLimit());
try {
- if (!commitTimeline.empty()) {
+ if (hoodieWriteConfig.getRecordSizeEstimationThreshold() > 0 && !commitTimeline.empty()) {
// Go over the reverse ordered commits to get a more recent estimate of average record size.
Iterator<HoodieInstant> instants = commitTimeline.getReverseOrderedInstants().iterator();
Review Comment:
> if (hoodieWriteConfig.getRecordSizeEstimationThreshold() > 0 && !commitTimeline.empty())
Shouldn't we first determine if the default value is adjusted (org.apache.hudi.config.HoodieCompactionConfig#_COPY_ON_WRITE_RECORD_SIZE_ESTIMATE)? I think this is the first priority.
Imagine, according to the logic adjusted above, that is, you can only set avgSize to a fixed 1024 (the default size) by adjusting the threshold, but not the ability to let users customize avgSize according to their personalized tasks.
If I have misunderstood, please let me know ,thanks
@danny0405 @honeyaya @codope @nsivabalan
--
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 #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on code in PR #7255:
URL: https://github.com/apache/hudi/pull/7255#discussion_r1027671915
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -372,7 +372,7 @@ protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, Hoodi
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
long fileSizeThreshold = (long) (hoodieWriteConfig.getRecordSizeEstimationThreshold() * hoodieWriteConfig.getParquetSmallFileLimit());
try {
- if (!commitTimeline.empty()) {
+ if (hoodieWriteConfig.getRecordSizeEstimationThreshold() > 0 && !commitTimeline.empty()) {
// Go over the reverse ordered commits to get a more recent estimate of average record size.
Iterator<HoodieInstant> instants = commitTimeline.getReverseOrderedInstants().iterator();
Review Comment:
> hi, we found that the result of totalBytesWritten/totalRecordsWritten is small when the last commit, but the next commit record number is very large
Got you, can you explain why the record size estimation from the first commit so small 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] honeyaya commented on a diff in pull request #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by GitBox <gi...@apache.org>.
honeyaya commented on code in PR #7255:
URL: https://github.com/apache/hudi/pull/7255#discussion_r1027630040
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -372,7 +372,7 @@ protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, Hoodi
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
long fileSizeThreshold = (long) (hoodieWriteConfig.getRecordSizeEstimationThreshold() * hoodieWriteConfig.getParquetSmallFileLimit());
try {
- if (!commitTimeline.empty()) {
+ if (hoodieWriteConfig.getRecordSizeEstimationThreshold() > 0 && !commitTimeline.empty()) {
// Go over the reverse ordered commits to get a more recent estimate of average record size.
Iterator<HoodieInstant> instants = commitTimeline.getReverseOrderedInstants().iterator();
Review Comment:
hi, we found that the result of totalBytesWritten/totalRecordsWritten is small when the last commit, but the next commit record number is very large, then the data files will become very large, in our case the data file will come to 600~700M.
More, our record size is fixed about 100M, so we don't want to use the above algorithm to help calculate avgsize and use the default value of the estimated record size which is set by users.
Although the default value of the estimation threshold is 1.0, in fact, I want to extend the usage of this property and then don't need to add another boolean property to control this, when we set it to less than 0, then could use the default size not using the totalBytesWritten/totalRecordsWritten.
--
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] bvaradar commented on pull request #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by "bvaradar (via GitHub)" <gi...@apache.org>.
bvaradar commented on PR #7255:
URL: https://github.com/apache/hudi/pull/7255#issuecomment-1493212690
@honeyaya : Interested in taking this PR to completion ?
--
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 #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on code in PR #7255:
URL: https://github.com/apache/hudi/pull/7255#discussion_r1027525224
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -372,7 +372,7 @@ protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, Hoodi
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
long fileSizeThreshold = (long) (hoodieWriteConfig.getRecordSizeEstimationThreshold() * hoodieWriteConfig.getParquetSmallFileLimit());
try {
- if (!commitTimeline.empty()) {
+ if (hoodieWriteConfig.getRecordSizeEstimationThreshold() > 0 && !commitTimeline.empty()) {
// Go over the reverse ordered commits to get a more recent estimate of average record size.
Iterator<HoodieInstant> instants = commitTimeline.getReverseOrderedInstants().iterator();
Review Comment:
The default value already greater than 0: say 1.0
--
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] bvaradar commented on pull request #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by "bvaradar (via GitHub)" <gi...@apache.org>.
bvaradar commented on PR #7255:
URL: https://github.com/apache/hudi/pull/7255#issuecomment-1465030892
@honeyaya : Changes look good. Can you add a test-case where getRecordSizeEstimationThreshold < 0
--
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 #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7255:
URL: https://github.com/apache/hudi/pull/7255#issuecomment-1321550376
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "98941b2fe0f47e0d3d0846cc7869c29eed4d090d",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13139",
"triggerID" : "98941b2fe0f47e0d3d0846cc7869c29eed4d090d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 98941b2fe0f47e0d3d0846cc7869c29eed4d090d Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13139)
<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] helpta commented on a diff in pull request #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by "helpta (via GitHub)" <gi...@apache.org>.
helpta commented on code in PR #7255:
URL: https://github.com/apache/hudi/pull/7255#discussion_r1111450040
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -372,7 +372,7 @@ protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, Hoodi
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
long fileSizeThreshold = (long) (hoodieWriteConfig.getRecordSizeEstimationThreshold() * hoodieWriteConfig.getParquetSmallFileLimit());
try {
- if (!commitTimeline.empty()) {
+ if (hoodieWriteConfig.getRecordSizeEstimationThreshold() > 0 && !commitTimeline.empty()) {
// Go over the reverse ordered commits to get a more recent estimate of average record size.
Iterator<HoodieInstant> instants = commitTimeline.getReverseOrderedInstants().iterator();
Review Comment:
> if (hoodieWriteConfig.getRecordSizeEstimationThreshold() > 0 && !commitTimeline.empty())
Shouldn't we first determine if the default value is adjusted (org.apache.hudi.config.HoodieCompactionConfig#_COPY_ON_WRITE_RECORD_SIZE_ESTIMATE)? I think this is the first priority.
Imagine, according to the logic adjusted above, that is, you can only set avgSize to a fixed 1024 (the default size) by adjusting the threshold, but not the ability to let users customize avgSize according to their personalized tasks.
If I have misunderstood, please let me know ,thanks
@danny0405 @honeyaya @codope @nsivabalan
--
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 #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by GitBox <gi...@apache.org>.
hudi-bot commented on PR #7255:
URL: https://github.com/apache/hudi/pull/7255#issuecomment-1321401362
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "98941b2fe0f47e0d3d0846cc7869c29eed4d090d",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13139",
"triggerID" : "98941b2fe0f47e0d3d0846cc7869c29eed4d090d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 98941b2fe0f47e0d3d0846cc7869c29eed4d090d Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=13139)
<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 #7255: [HUDI-5250] use the estimate record size when estimation threshold is l…
Posted by GitBox <gi...@apache.org>.
danny0405 commented on code in PR #7255:
URL: https://github.com/apache/hudi/pull/7255#discussion_r1027525224
##########
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/table/action/commit/UpsertPartitioner.java:
##########
@@ -372,7 +372,7 @@ protected static long averageBytesPerRecord(HoodieTimeline commitTimeline, Hoodi
long avgSize = hoodieWriteConfig.getCopyOnWriteRecordSizeEstimate();
long fileSizeThreshold = (long) (hoodieWriteConfig.getRecordSizeEstimationThreshold() * hoodieWriteConfig.getParquetSmallFileLimit());
try {
- if (!commitTimeline.empty()) {
+ if (hoodieWriteConfig.getRecordSizeEstimationThreshold() > 0 && !commitTimeline.empty()) {
// Go over the reverse ordered commits to get a more recent estimate of average record size.
Iterator<HoodieInstant> instants = commitTimeline.getReverseOrderedInstants().iterator();
Review Comment:
The default value already greater than 0
--
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