-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Avoid prefetch too much data when offloading data to HDFS #6717
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
Conversation
|
@congbobo184 is it possible for you to review this PR? |
.../org/apache/bookkeeper/mledger/offload/filesystem/impl/FileSystemManagedLedgerOffloader.java
Outdated
Show resolved
Hide resolved
.../org/apache/bookkeeper/mledger/offload/filesystem/impl/FileSystemManagedLedgerOffloader.java
Show resolved
Hide resolved
|
@congbobo184 Is it possible for you to review the PR again? |
.../org/apache/bookkeeper/mledger/offload/filesystem/impl/FileSystemManagedLedgerOffloader.java
Outdated
Show resolved
Hide resolved
.../org/apache/bookkeeper/mledger/offload/filesystem/impl/FileSystemManagedLedgerOffloader.java
Outdated
Show resolved
Hide resolved
|
@congbobo184 your comment makes sense. I have update the code. Thanks. Any more modification for me? |
|
/pulsarbot run-failure-checks |
|
@congbobo184 need I handle the CI-Integration test fail? I think it is not related to this PR |
|
/pulsarbot run-failure-checks |
|
you don't need to handle the CI-Integration. |
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
.../org/apache/bookkeeper/mledger/offload/filesystem/impl/FileSystemManagedLedgerOffloader.java
Outdated
Show resolved
Hide resolved
|
Nice work! 👍 @pheecian |
Fixes apache#6692 avoid prefetch too much data when offloading, which may lead to OOM; fix object not close issue, which is also mentioned by congbobo184 apache#6697 *Explain here the context, and why you're making that change. What is the problem you're trying to solve.* *If `yes` was chosen, please highlight the changes* - Dependencies (does it add or upgrade a dependency): (no) - The public API: (no) - The schema: (no) - The default values of configurations: (no) - The wire protocol: (no) - The rest endpoints: (no) - The admin cli options: (no) - Anything that affects deployment: (no) - Does this pull request introduce a new feature? (no)
Fixes #6692 ### Motivation avoid prefetch too much data when offloading, which may lead to OOM; fix object not close issue, which is also mentioned by congbobo184 #6697 *Explain here the context, and why you're making that change. What is the problem you're trying to solve.* ### Does this pull request potentially affect one of the following parts: *If `yes` was chosen, please highlight the changes* - Dependencies (does it add or upgrade a dependency): (no) - The public API: (no) - The schema: (no) - The default values of configurations: (no) - The wire protocol: (no) - The rest endpoints: (no) - The admin cli options: (no) - Anything that affects deployment: (no) ### Documentation - Does this pull request introduce a new feature? (no) (cherry picked from commit 514b6af)
…ache#6717)" This reverts commit 74c668a.
Fixes apache#6692 ### Motivation avoid prefetch too much data when offloading, which may lead to OOM; fix object not close issue, which is also mentioned by congbobo184 apache#6697 *Explain here the context, and why you're making that change. What is the problem you're trying to solve.* ### Does this pull request potentially affect one of the following parts: *If `yes` was chosen, please highlight the changes* - Dependencies (does it add or upgrade a dependency): (no) - The public API: (no) - The schema: (no) - The default values of configurations: (no) - The wire protocol: (no) - The rest endpoints: (no) - The admin cli options: (no) - Anything that affects deployment: (no) ### Documentation - Does this pull request introduce a new feature? (no)
Fixes #6692
Motivation
avoid prefetch too much data when offloading, which may lead to OOM;
fix object not close issue, which is also mentioned by congbobo184 #6697
Explain here the context, and why you're making that change. What is the problem you're trying to solve.
Does this pull request potentially affect one of the following parts:
If
yeswas chosen, please highlight the changesDocumentation