Skip to content

[HUDI-4578] Reduce the scope and duration of holding checkpoint lock …#6337

Open
XuQianJin-Stars wants to merge 1 commit intoapache:masterfrom
XuQianJin-Stars:HUDI-4578
Open

[HUDI-4578] Reduce the scope and duration of holding checkpoint lock …#6337
XuQianJin-Stars wants to merge 1 commit intoapache:masterfrom
XuQianJin-Stars:HUDI-4578

Conversation

@XuQianJin-Stars
Copy link
Contributor

@XuQianJin-Stars XuQianJin-Stars commented Aug 9, 2022

Change Logs

Now the scope of the lock is too large.

image

image

Impact

Describe any public API or user-facing feature change or any performance impact.

Risk level: none | low | medium | high

Choose one. If medium or high, explain what verification was done to mitigate the risks.

Contributor's checklist

  • Read through contributor's guide
  • Change Logs and Impact were stated clearly
  • Adequate tests were added if applicable
  • CI passed


// only need to hold the checkpoint lock when emitting the splits
start = System.currentTimeMillis();
synchronized (context.getCheckpointLock()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

use checkpointLock ?

@danny0405
Copy link
Contributor

How much gains do we have after this change ? Guess the checkpoint lock would not block frequently ?

@XuQianJin-Stars
Copy link
Contributor Author

How much gains do we have after this change ? Guess the checkpoint lock would not block frequently ?

To fundamentally solve the problem of stream read timeout and lock, it is necessary to optimize the number of MergeOnReadInputSplit generated and read.

@hudi-bot
Copy link
Collaborator

hudi-bot commented Aug 9, 2022

CI report:

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

@yihua yihua added priority:high Significant impact; potential bugs writer-core engine:flink Flink integration labels Sep 6, 2022
@nsivabalan nsivabalan added the release-0.12.2 Patches targetted for 0.12.2 label Dec 6, 2022
@codope codope removed the release-0.12.2 Patches targetted for 0.12.2 label Dec 7, 2022
@github-actions github-actions bot added the size:S PR with lines of changes in (10, 100] label Feb 26, 2024
Copy link
Contributor

@yihua yihua left a comment

Choose a reason for hiding this comment

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

@danny0405 is this still beneficial on Flink?

Copy link
Contributor

@yihua yihua left a comment

Choose a reason for hiding this comment

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

@danny0405 @cshuo is this PR still needed after Hudi 1.1 uses async instant time generation in Flink?

@danny0405
Copy link
Contributor

It is still valid issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine:flink Flink integration priority:high Significant impact; potential bugs size:S PR with lines of changes in (10, 100]

Projects

Status: 🚧 Needs Repro
Status: 🔖 Ready for review

Development

Successfully merging this pull request may close these issues.

8 participants