Skip to content

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

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
Merged
Technoboy- merged 2 commits into from
Aug 3, 2022
Merged

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

Technoboy- merged 2 commits into from
Aug 3, 2022

Conversation

@Nicklee007
Copy link
Contributor

@Nicklee007 Nicklee007 commented Aug 2, 2022 •
edited
Loading

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
@Nicklee007 Nicklee007 requested a review from Jason918 August 2, 2022 11:40
@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

mattisonchao commented Aug 4, 2022

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

Nicklee007 commented Aug 5, 2022

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 commented Aug 5, 2022

@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

Nicklee007 commented Aug 5, 2022

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

mattisonchao commented Aug 10, 2022

Move release/2.9.4 to #16958

@github-actions
Copy link

github-actions bot commented Aug 10, 2022

@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

@hezhangjian hezhangjian hezhangjian 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.

10 participants

@Nicklee007 @Jason918 @mattisonchao @Technoboy- @codelipenghui @hezhangjian @RobertIndie @coderzc @liudezhi2098 @BewareMyPower