Skip to content

Conversation

poorbarcode
Copy link
Contributor

Fixes #24445

Main Issue: #24445

Motivation

PR #23052 fixed the issue caused by creating TopicName and adding items into the queue of TopicName Cache.

Issue: PR added a clarification for Pulsar Broker, but forgot to add the clarification for Pulsar Proxy, Pulsar WebSocket Service, and Pulsar Client.

Modifications

Documentation

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

Matching PR in forked repository

PR in forked repository: x

@poorbarcode poorbarcode added this to the 4.1.0 milestone Jun 24, 2025
@poorbarcode poorbarcode self-assigned this Jun 24, 2025
@poorbarcode poorbarcode added type/bug The PR fixed a bug or issue reported a bug release/3.0.13 release/4.0.6 release/3.3.8 labels Jun 24, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jun 24, 2025
@poorbarcode poorbarcode changed the title [fix][prox]Pulsar Proxy OOM because forget to clear topic name cache [fix][prox]Pulsar Proxy OOMs because it forgets to clear the topic name cache Jun 24, 2025
@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@poorbarcode poorbarcode changed the title [fix][prox]Pulsar Proxy OOMs because it forgets to clear the topic name cache [fix][proxy]Pulsar Proxy OOMs because it forgets to clear the topic name cache Jun 24, 2025
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

This is a competing PR to fix the issue. I've already submitted #24457 and provided my argumentation in #24457 (comment) .

@poorbarcode poorbarcode requested a review from coderzc June 25, 2025 04:19
@lhotari lhotari dismissed their stale review June 25, 2025 10:43

Unblocking this PR since we can come back to the #24457 while optimizing topic pattern related memory usage. String deduplication is mostly relevant for those use cases.

@poorbarcode
Copy link
Contributor Author

poorbarcode commented Jun 25, 2025

@lhotari

Unblocking this PR since we can come back to the #24457 while optimizing topic pattern related memory usage. String deduplication is mostly relevant for those use cases.

Yes, let's first solve the problem with the original plan, and then discuss the rationality of the optimization

Thanks ❤️

Copy link
Member

@lhotari lhotari 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

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

Please hold this PR for some time. I'm working on a better solution motivated by my PR today: #24463

I will push it ASAP.

@BewareMyPower
Copy link
Contributor

@lhotari @poorbarcode @coderzc I pushed a new solution that:

  • has better performance with a channel based topic cache
  • does not introduce any new configurations or scheduled task

PTAL #24465

@3pacccccc
Copy link
Contributor

I have a question, under what scenario will evictCacheByScheduledTask be false? I see you set it to true in BrokerService、WebSocketService and ProxyService

Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

Though it's already the same behavior as broker is, I'm wondering how can it address the following case?

LOOP:
   Create a topic with a random name
   Do some send and receive
   Delete this topic

The cache could easily be very large and cause OOM.

I believe the broker has the same issue as well.

@poorbarcode
Copy link
Contributor Author

I pushed a new solution that:

has better performance with a channel based topic cache
does not introduce any new configurations or scheduled task
PTAL #24465

Close the PR

@poorbarcode poorbarcode closed this Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs ready-to-test release/3.0.13 release/3.3.8 release/4.0.6 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Memory leak in Pulsar Proxy with TopicName
5 participants