Skip to content

[HUDI-3834] Fixing performance hits in reading Column Stats Index#5266

Merged
nsivabalan merged 15 commits intoapache:masterfrom
onehouseinc:ak/col-stat-perf-fix
Apr 10, 2022
Merged

[HUDI-3834] Fixing performance hits in reading Column Stats Index#5266
nsivabalan merged 15 commits intoapache:masterfrom
onehouseinc:ak/col-stat-perf-fix

Conversation

@alexeykudinkin
Copy link
Contributor

Tips

What is the purpose of the pull request

Fixing performance hits in reading Column Stats Index:

  1. [HUDI-3834] There's substantial performance degradation in Avro 1.10 default generated Builder classes: they by default rely on SpecificData.getForSchema that load corresponding model's class using reflection, which takes a hit when executed on the hot-path (this was bringing overall runtime to read full Column Stats Index of 800k records to 60s, whereas now it's taking mere 3s)

  2. Addressing memory churn by over-used Hadoop's Path creation: Path ctor is not a lightweight sequence and produces quite a bit of memory churn adding pressure on GC. Cleaning such avoidable allocations up to make sure there's no unnecessarily added pressure on GC.

Brief change log

See above

Verify this pull request

This pull request is already covered by existing tests, such as (please describe tests).

Committer checklist

  • Has a corresponding JIRA in PR title & commit

  • Commit message is descriptive of the change

  • CI is green

  • Necessary doc changes done or have another open PR

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

Copy link
Contributor

@nsivabalan nsivabalan left a comment

Choose a reason for hiding this comment

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

Really good job on the finding and the fix.

* @return Base path
* @deprecated please use {@link #getBasePathV2()}
*/
@Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

I see we are using getBasePath() in our baseRelation classes. do we need to fix them to getBasePathV2() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should slowly rollover all uses of getBasePath into getBasePathV2 and then rename it

// reads/writes to references are always atomic (including 64-bit JVMs)
// https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.7
private volatile String fileName;
private volatile String s;
Copy link
Contributor

Choose a reason for hiding this comment

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

minor. s -> path.

@nsivabalan nsivabalan added the priority:blocker Production down; release blocker label Apr 9, 2022
@nsivabalan nsivabalan self-assigned this Apr 9, 2022
Copy link
Contributor

@nsivabalan nsivabalan left a comment

Choose a reason for hiding this comment

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

LGTM

@nsivabalan
Copy link
Contributor

@alexeykudinkin : there are some CI failures. can you please check it out

@alexeykudinkin
Copy link
Contributor Author

@hudi-bot run azure

@hudi-bot
Copy link
Collaborator

hudi-bot commented Apr 9, 2022

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@nsivabalan nsivabalan merged commit 7a9d48d into apache:master Apr 10, 2022
xushiyan pushed a commit that referenced this pull request Apr 14, 2022
)

Fixing performance hits in reading Column Stats Index:

[HUDI-3834] There's substantial performance degradation in Avro 1.10 default generated Builder classes: they by default rely on SpecificData.getForSchema that load corresponding model's class using reflection, which takes a hit when executed on the hot-path (this was bringing overall runtime to read full Column Stats Index of 800k records to 60s, whereas now it's taking mere 3s)

Addressing memory churn by over-used Hadoop's Path creation: Path ctor is not a lightweight sequence and produces quite a bit of memory churn adding pressure on GC. Cleaning such avoidable allocations up to make sure there's no unnecessarily added pressure on GC.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:blocker Production down; release blocker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants