Skip to content
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.

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

[BugFix] close hudi fsview to avoid resource leak #52738

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

dirtysalt
Copy link
Contributor

@dirtysalt dirtysalt commented Nov 8, 2024

Why I'm doing:

Hudi bitcask temp files descriptors are leaked.

What I'm doing:

Close hudi filesystem view when we don't need it.

Fixes #52737


This fix has a potential drawback but we can avoid that.

Before this PR, we create fileststem view when the request comes, and we only create the fsview for once. However, we don't know when to close this fsview. And fsview is left in opened state, and resource leak happens.

image

To fix this problem, we have to close this fsview when no requests need this fsview. We can use reference count to solve it. However we don't know how many total requests will be issued. Because some request will be cached without using this fsview, we don't know how many requests will use fsview in advance.

image

However there is a potential drawback. If some requests are not overlapped, fsview will be created and closed for multiple times. In practice it seldom happens.

image

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 3.3
    • 3.2
    • 3.1
    • 3.0
    • 2.5

@dirtysalt dirtysalt requested a review from a team as a code owner November 8, 2024 06:47
gengjun-git
gengjun-git previously approved these changes Nov 8, 2024
nshangyiming
nshangyiming previously approved these changes Nov 8, 2024
@dirtysalt dirtysalt changed the title [BugFix] close hudi fsview to avoid resource leak [WIP][BugFix] close hudi fsview to avoid resource leak Nov 8, 2024
Copy link

@emilov emilov left a comment

Choose a reason for hiding this comment

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

Thanks for catching this!

Copy link

sonarcloud bot commented Nov 10, 2024

@dirtysalt dirtysalt changed the title [WIP][BugFix] close hudi fsview to avoid resource leak [BugFix] close hudi fsview to avoid resource leak Nov 10, 2024
Copy link

[Java-Extensions Incremental Coverage Report]

pass : 0 / 0 (0%)

Copy link

[FE Incremental Coverage Report]

pass : 29 / 30 (96.67%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/connector/hudi/HudiRemoteFileIO.java 22 23 95.65% [117]
🔵 com/starrocks/connector/RemoteFileScanContext.java 7 7 100.00% []

Copy link

[BE Incremental Coverage Report]

pass : 0 / 0 (0%)

@dirtysalt dirtysalt enabled auto-merge (squash) November 11, 2024 02:59
@dirtysalt dirtysalt merged commit ec8eae4 into StarRocks:main Nov 11, 2024
69 of 71 checks passed
Copy link

@Mergifyio backport branch-3.3

@github-actions github-actions bot removed the 3.3 label Nov 11, 2024
Copy link

@Mergifyio backport branch-3.2

@github-actions github-actions bot removed the 3.2 label Nov 11, 2024
Copy link
Contributor

mergify bot commented Nov 11, 2024

backport branch-3.3

✅ Backports have been created

Copy link
Contributor

mergify bot commented Nov 11, 2024

backport branch-3.2

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Nov 11, 2024
Signed-off-by: yanz <[email protected]>
(cherry picked from commit ec8eae4)

# Conflicts:
#	fe/fe-core/src/main/java/com/starrocks/connector/RemoteFileScanContext.java
#	fe/fe-core/src/main/java/com/starrocks/connector/RemotePathKey.java
#	fe/fe-core/src/main/java/com/starrocks/connector/hudi/HudiRemoteFileIO.java
mergify bot pushed a commit that referenced this pull request Nov 11, 2024
Signed-off-by: yanz <[email protected]>
(cherry picked from commit ec8eae4)

# Conflicts:
#	fe/fe-core/src/main/java/com/starrocks/connector/RemoteFileScanContext.java
#	fe/fe-core/src/main/java/com/starrocks/connector/RemotePathKey.java
#	fe/fe-core/src/main/java/com/starrocks/connector/hudi/HudiRemoteFileIO.java
wanpengfei-git pushed a commit that referenced this pull request Nov 11, 2024
wanpengfei-git pushed a commit that referenced this pull request Nov 11, 2024
@dirtysalt dirtysalt deleted the fix-hudi-fs-close branch November 11, 2024 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hudi tmp BITCASK takes too many file descriptors
5 participants