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 QueueEntry recycle problem. #3747

Merged
merged 2 commits into from
Feb 3, 2023

Conversation

horizonzy
Copy link
Member

Descriptions of the changes in this PR:
In the QueueEntry recycle, it only recycles itself to the object pool, but didn't reset some properties.
Like entry, cb, etc. We should reset the filed before recycles itself.

Copy link
Contributor

@hangc0276 hangc0276 left a comment

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Contributor

@StevenLuMT StevenLuMT left a comment

Choose a reason for hiding this comment

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

Great work

Copy link
Member

@wenbingshen wenbingshen left a comment

Choose a reason for hiding this comment

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

nice work!

@hangc0276 hangc0276 merged commit 901f76c into apache:master Feb 3, 2023
callbackTime.addLatency(MathUtils.elapsedNanos(startTime), TimeUnit.NANOSECONDS);
recycle();
Copy link
Member

Choose a reason for hiding this comment

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

@horizonzy This change wasn't mentioned in the PR description. Why was this missing before? Was the recycling completely unused before? /cc @hangc0276

Copy link
Member Author

@horizonzy horizonzy Feb 3, 2023

Choose a reason for hiding this comment

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

Cause we already reset callbackTime to null. And we should ensure do all works before recycle self, instead of addLatency after recycle self.

hangc0276 pushed a commit that referenced this pull request Feb 8, 2023
Descriptions of the changes in this PR:
In the QueueEntry recycle, it only recycles itself to the object pool, but didn't reset some properties.
Like entry, cb, etc. We should reset the filed before recycles itself.

(cherry picked from commit 901f76c)
zymap pushed a commit that referenced this pull request Feb 16, 2023
Descriptions of the changes in this PR:
In the QueueEntry recycle, it only recycles itself to the object pool, but didn't reset some properties.
Like entry, cb, etc. We should reset the filed before recycles itself.

(cherry picked from commit 901f76c)
nicoloboschi pushed a commit to datastax/bookkeeper that referenced this pull request Mar 13, 2023
Descriptions of the changes in this PR:
In the QueueEntry recycle, it only recycles itself to the object pool, but didn't reset some properties.
Like entry, cb, etc. We should reset the filed before recycles itself.

(cherry picked from commit 901f76c)
(cherry picked from commit e841a3d)
nicoloboschi pushed a commit to datastax/bookkeeper that referenced this pull request Mar 13, 2023
Descriptions of the changes in this PR:
In the QueueEntry recycle, it only recycles itself to the object pool, but didn't reset some properties.
Like entry, cb, etc. We should reset the filed before recycles itself.

(cherry picked from commit 901f76c)
(cherry picked from commit e841a3d)
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