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

[fix][offload] Fix OOM in tiered storage, caused by unbounded offsets cache #22679

Merged
merged 2 commits into from
May 9, 2024

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented May 8, 2024

Motivation

In #16417 a cache was added for caching offsets. The problem with this implementation is that in some cases, the
memory consumption can go very high and cause OOM.

Modifications

  • Move the cache to the org.apache.bookkeeper.mledger.offload.jcloud.JCloudLedgerOffloaderFactory level and share it across all offloaded ledgers.
  • Add a default maximum size of 1M entries. This consumes about 60MB of heap at maximum.
    • add system property pulsar.jclouds.readhandleimpl.offsetsscache.max.size to adjust this, however there shouldn't be a need to tune it.
  • Add scheduler to run the cleanup job periodically so that the TTL limit will be enforced in all cases.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@Technoboy- Technoboy- added this to the 3.4.0 milestone May 9, 2024
Copy link
Contributor

@Technoboy- Technoboy- left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Lgtm

@lhotari lhotari merged commit 566330c into apache:master May 9, 2024
49 of 50 checks passed
lhotari added a commit that referenced this pull request May 9, 2024
… cache (#22679)

Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit 566330c)
@lhotari
Copy link
Member Author

lhotari commented May 9, 2024

Follow up to fix a small issue: #22683

lhotari added a commit that referenced this pull request May 9, 2024
… cache (#22679)

Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit 566330c)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request May 13, 2024
… cache (apache#22679)

Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit 566330c)
(cherry picked from commit f091bcb)
coderzc pushed a commit that referenced this pull request May 14, 2024
… cache (#22679)

Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit 566330c)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 16, 2024
… cache (apache#22679)

Co-authored-by: Jiwe Guo <technoboy@apache.org>
(cherry picked from commit 566330c)
(cherry picked from commit f091bcb)
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.

None yet

7 participants