Skip to content

[fix][broke] Fix ShadowReplicator source entry buffer leak#25732

Merged
nodece merged 2 commits into
apache:masterfrom
void-ptr974:fix-shadow-replicator-bytebuf-leak
May 14, 2026
Merged

[fix][broke] Fix ShadowReplicator source entry buffer leak#25732
nodece merged 2 commits into
apache:masterfrom
void-ptr974:fix-shadow-replicator-bytebuf-leak

Conversation

@void-ptr974
Copy link
Copy Markdown
Contributor

@void-ptr974 void-ptr974 commented May 10, 2026

fix #25257

Motivation

ShadowReplicator was retaining the source entry headersAndPayload buffer before sending the replicated message. Unlike geo replication, shadow replication deserializes source entries
with an empty payload, so the producer send path does not own or release the original source entry buffer.

This left one extra reference after the send callback released the Entry, causing Netty leak detector reports like:

LEAK: ByteBuf.release() was not called

Changes

  • Remove the unnecessary headersAndPayload.retain() in ShadowReplicator.
  • Add a regression test that exercises the ShadowReplicator send path and verifies the source entry ByteBuf refCnt reaches 0 after replication completes.

Verification

./gradlew :pulsar-broker:test --tests org.apache.pulsar.broker.service.persistent.ShadowReplicatorTest

Also verified the new test fails before the fix with expected [0] but found [1].

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

  ShadowReplicator deserializes source entries with an empty payload, so the
  producer send path does not own or release the source entry buffer. Remove the
  extra retain on headersAndPayload to let the send callback's Entry release fully
  release the buffer.

  Add a regression test that exercises the ShadowReplicator send path and verifies
  the source entry ByteBuf refCnt reaches zero after replication completes.
@void-ptr974
Copy link
Copy Markdown
Contributor Author

@dlg99 @Jason918 @lhotari PTAL

@void-ptr974
Copy link
Copy Markdown
Contributor Author

CI check pass in my local repo: https://github.com/void-ptr974/pulsar/actions/runs/25626436624

@nodece nodece requested review from dao-jun and lhotari May 12, 2026 03:09
Copy link
Copy Markdown
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM. It's introduced by the improvement in #23236

@Jason918
Copy link
Copy Markdown
Contributor

@RobertIndie PTAL. Seems related to #23236

@Jason918 Jason918 requested a review from RobertIndie May 14, 2026 07:45
@nodece nodece merged commit ac53a9e into apache:master May 14, 2026
80 of 82 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] ShadowTopicTest: "LEAK: ByteBuf.release() was not called"

3 participants