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.

Sign up for GitHub

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

Jump to bottom

[fix][client]Fix MaxQueueSize semaphore release leak in createOpSendMsg #16915

Merged
merged 2 commits into from Aug 3, 2022
Merged

[fix][client]Fix MaxQueueSize semaphore release leak in createOpSendMsg #16915

merged 2 commits into from Aug 3, 2022

Conversation

Nicklee007
Copy link
Contributor

@Nicklee007 Nicklee007 commented Aug 2, 2022 •
edited

Motivation

Release MaxQueueSize semaphore when invalid message, the related PR #16835 only release the memory usage, but the semaphore also need release in here.
The fixed unit test can reproduce it, which only enableBatching can invoke the createOpSendMsg in BatchMessageContainerImpl.

Modifications

  1. add semaphore release;
  2. fix unit test;

Documentation

  • doc-not-needed
    (Please explain why)

@Technoboy- Technoboy- added this to the 2.11.0 milestone Aug 2, 2022
@Technoboy- Technoboy- merged commit d95f6cf into apache:master Aug 3, 2022
Jason918
Jason918 reviewed Aug 3, 2022
View reviewed changes
Copy link
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

@Jason918
Copy link
Contributor

Jason918 commented Aug 4, 2022

There is a conflict when direct merge this to branch 2.7
@Nicklee007 Please help open another PR to merge this to branch 2.7

@mattisonchao
Copy link
Member

Please also help to cherry-pick to branch 2.9

Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Aug 5, 2022
cherry pick apache#16915 to branch 2.7
4a529b7
@Nicklee007 Nicklee007 mentioned this pull request Aug 5, 2022
Merged
1 task
@Nicklee007
Copy link
Contributor Author

Please also help to cherry-pick to branch 2.9

@mattisonchao ok, I'll cherry-pick it to branch 2.9.

@Nicklee007
Copy link
Contributor Author

@Nicklee007 Please help open another PR to merge this to branch 2.7

@Jason918 branch 2.7 cherry-pick PR #16957 , PTAL.

Nicklee007 added a commit to Nicklee007/pulsar that referenced this pull request Aug 5, 2022
@Nicklee007
cherry pick apache#16915 to branch 2.9
3e66e55
@Jason918
Copy link
Contributor

Jason918 commented Aug 5, 2022

Move release/2.7.5 to #16957

@github-actions
Copy link

github-actions bot commented Aug 5, 2022

@Nicklee007 Please provide a correct documentation label for your PR.
Instructions see Pulsar Documentation Label Guide.

Nicklee007 added a commit to Nicklee007/pulsar that referenced this pull request Aug 5, 2022
@Nicklee007
cherry pick apache#16915 to branch 2.9
1ec9e3d
@Nicklee007 Nicklee007 mentioned this pull request Aug 5, 2022
Merged
1 task
@Nicklee007
Copy link
Contributor Author

Please also help to cherry-pick to branch 2.9

@mattisonchao branch 2.9 cherry-pick PR #16958, PTAL.

BewareMyPower pushed a commit that referenced this pull request Aug 5, 2022
@Nicklee007 @BewareMyPower
[fix][client]Fix MaxQueueSize semaphore release leak in createOpSendM…
59339c4 
…sg (#16915)

(cherry picked from commit d95f6cf)
@BewareMyPower BewareMyPower added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Aug 5, 2022
@github-actions
Copy link

github-actions bot commented Aug 5, 2022

@Nicklee007 Please provide a correct documentation label for your PR.
Instructions see Pulsar Documentation Label Guide.

Jason918 pushed a commit that referenced this pull request Aug 7, 2022
@Nicklee007
cherry pick #16915 to branch 2.7 (#16957)
85b6b63 
Co-authored-by: nicklixinyang <nicklixinyang@didiglobal.com>
codelipenghui pushed a commit that referenced this pull request Aug 8, 2022
@Nicklee007 @codelipenghui
[fix][client]Fix MaxQueueSize semaphore release leak in createOpSendM…
70f1a03 
…sg (#16915)

(cherry picked from commit d95f6cf)
@mattisonchao
Copy link
Member

Move release/2.9.4 to #16958

@github-actions
Copy link

@Nicklee007 Please provide a correct documentation label for your PR.
Instructions see Pulsar Documentation Label Guide.

nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Aug 16, 2022
@Nicklee007 @nicoloboschi
[fix][client]Fix MaxQueueSize semaphore release leak in createOpSendM…
eeb2ed2 
…sg (apache#16915)

(cherry picked from commit d95f6cf)
(cherry picked from commit 70f1a03)
@gaozhangmin gaozhangmin mentioned this pull request Aug 31, 2022
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jason918 Jason918 Jason918 left review comments

@Technoboy- Technoboy- Technoboy- approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@shoothzj shoothzj shoothzj approved these changes

@RobertIndie RobertIndie RobertIndie approved these changes

@coderzc coderzc coderzc approved these changes

@liudezhi2098 liudezhi2098 liudezhi2098 approved these changes

Assignees

@Nicklee007 Nicklee007

Labels
area/client cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.10 doc-label-missing release/2.8.4 release/2.10.2 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Milestone
2.11.0
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@Nicklee007 @Jason918 @mattisonchao @Technoboy- @codelipenghui @shoothzj @RobertIndie @coderzc @liudezhi2098 @BewareMyPower