Skip to content

[HUDI-605] Avoid calculating the size of schema redundantly #1317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
leesf merged 2 commits into from
Feb 12, 2020
Merged

[HUDI-605] Avoid calculating the size of schema redundantly #1317

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e46bdd
[HUDI-605] Avoid calculating the size of schema redundantly
lamberken Feb 10, 2020
9784ca2
revert org.apache.hudi.common.util.collection.DiskBasedMap
lamberken Feb 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions hudi-common/src/main/java/org/apache/hudi/common/util/HoodieRecordSizeEstimator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,10 @@ public class HoodieRecordSizeEstimator<T extends HoodieRecordPayload> implements

private static final Logger LOG = LogManager.getLogger(HoodieRecordSizeEstimator.class);

// Schema used to get GenericRecord from HoodieRecordPayload then convert to bytes and vice-versa
private final Schema schema;
private final long sizeOfSchema;

public HoodieRecordSizeEstimator(Schema schema) {
this.schema = schema;
sizeOfSchema = ObjectSizeCalculator.getObjectSize(schema);
}

@Override
Expand All @@ -49,8 +48,9 @@ public long sizeEstimate(HoodieRecord<T> hoodieRecord) {
// note the sizes and differences. A correct estimation in such cases is handled in
/** {@link ExternalSpillableMap} **/
long sizeOfRecord = ObjectSizeCalculator.getObjectSize(hoodieRecord);
long sizeOfSchema = ObjectSizeCalculator.getObjectSize(schema);
LOG.info("SizeOfRecord => " + sizeOfRecord + " SizeOfSchema => " + sizeOfSchema);
if (LOG.isDebugEnabled()) {
LOG.debug("SizeOfRecord => " + sizeOfRecord + " SizeOfSchema => " + sizeOfSchema);
}
Comment on lines +51 to +53
Copy link
Contributor

@leesf leesf Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be still keep to LOG.info or Any reason to change to debug?

Copy link
Member Author

@lamberken lamberken Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing this. Use LOG.info will print each time when you put an element, LOG.debug is better.

Copy link
Member Author

@lamberken lamberken Feb 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More, HoodieRecordSizeEstimator is mainly used in ExternalSpillableMap, ExternalSpillableMap#put has logged it.

if (shouldEstimatePayloadSize && estimatedPayloadSize == 0) {
  // At first, use the sizeEstimate of a record being inserted into the spillable map.
  // Note, the converter may over estimate the size of a record in the JVM
  this.estimatedPayloadSize = keySizeEstimator.sizeEstimate(key) + valueSizeEstimator.sizeEstimate(value);
  LOG.info("Estimated Payload size => " + estimatedPayloadSize);
}

return sizeOfRecord;
}
}