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

Use doOnDiscard to release cached Payloads #777

Merged
merged 18 commits into from
Apr 21, 2020
Merged

Use doOnDiscard to release cached Payloads #777

merged 18 commits into from
Apr 21, 2020

Conversation

OlegDokuka
Copy link
Member

In this PR I wanna introduce extra hooks to ensure we capture and release all the discarded payloads.

In this case, I use doOnDiscard operator in order to release dropped elements for every stream we have.

Also, just for now, I created an extra class for Queue wrapping in order to ensure that UnicastProcesssor does not leak elements on cancellation

Please note, this PR does not address all the cases and a user still MUST install Hooks.onNextDropped to ensure there are no leaks if a payload goes beyond rsocket internals scope.

Signed-off-by: Oleh Dokuka shadowgun@i.ua

@OlegDokuka OlegDokuka self-assigned this Apr 10, 2020
@OlegDokuka OlegDokuka added the bug label Apr 10, 2020
@OlegDokuka OlegDokuka added this to the 1.0 milestone Apr 10, 2020
@OlegDokuka OlegDokuka linked an issue Apr 10, 2020 that may be closed by this pull request
ZERO_COPY and memory leaks #733
Closed
@OlegDokuka OlegDokuka force-pushed the bugfix/track-discarded branch from cba41f0 to 626dd81 Compare April 10, 2020 15:33
@OlegDokuka OlegDokuka mentioned this pull request Apr 12, 2020
Closed
@OlegDokuka OlegDokuka added the blocked Blocked due to another dependency label Apr 13, 2020
@OlegDokuka
Copy link
Member Author

Blocked due to reactor/reactor-core#2114

@OlegDokuka OlegDokuka mentioned this pull request Apr 14, 2020
Closed
@OlegDokuka OlegDokuka marked this pull request as ready for review April 18, 2020 07:55
@OlegDokuka OlegDokuka modified the milestones: 1.0, 1.0 RC7 Apr 18, 2020
@OlegDokuka OlegDokuka removed a link to an issue Apr 18, 2020
ZERO_COPY and memory leaks #733
Closed
@OlegDokuka OlegDokuka removed the blocked Blocked due to another dependency label Apr 18, 2020
@OlegDokuka OlegDokuka force-pushed the bugfix/track-discarded branch from d761e59 to 942a33d Compare April 18, 2020 08:05
rstoyanchev
rstoyanchev requested changes Apr 18, 2020
View reviewed changes
Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

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

Looks good, very important change!

A few comments and questions below. Also update copyrights and add one for CleanOnClearQueueDecorator.

rsocket-core/src/main/java/io/rsocket/core/CleanOnClearQueueDecorator.java Outdated Show resolved Hide resolved
rsocket-core/src/main/java/io/rsocket/core/CleanOnClearQueueDecorator.java Outdated Show resolved Hide resolved
rsocket-core/src/test/java/io/rsocket/buffer/LeaksTrackingByteBufAllocator.java Outdated Show resolved Hide resolved
rsocket-core/src/test/java/io/rsocket/core/RSocketRequesterTest.java Outdated Show resolved Hide resolved
@OlegDokuka OlegDokuka force-pushed the bugfix/track-discarded branch 3 times, most recently from f21ce2e to 13a4fbb Compare April 20, 2020 22:03
@OlegDokuka OlegDokuka requested a review from rstoyanchev April 21, 2020 11:07
@OlegDokuka
provides extra hooks to ensure we capture all discarded elements
e0c65c4 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
provides leaks tracking tooling
1f9aa83 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
provides leaks tracking tests and tooling
82c4af1 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
more tests
cdb2846 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
provides mechanism for terminates queue on calling clear
3babea7 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
provides workaround for FluxPublishOn to ensure that all elements are…
d91bafc 
… released in case of racing

Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
provides more tests
7815825 
part of the tests are on racing (ignored for now)
another few on verification that elements are discarded properly

Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
provide fixes to RequestChannel responder and related tests.
263692f 
Ensures there is no leaks in RSocketRequesterTest and RSocketResponder tests

Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
tries to migrate to junit 5
e80f6a1 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
fixes leaks in tests
f7dd4de 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
optimizes discarded/dropped BB consumption and releasing
6945ba6 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka OlegDokuka force-pushed the bugfix/track-discarded branch 3 times, most recently from 0a25208 to 4743921 Compare April 21, 2020 12:10
@OlegDokuka
fixes javadocs
1595288 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka OlegDokuka force-pushed the bugfix/track-discarded branch from 4743921 to 1595288 Compare April 21, 2020 12:14
@OlegDokuka
removes hooks from Decoder
f4ed1f1 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
fixes format
8e311f0 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
rollbacks some fixes that should be delivered separately
274cd08 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
rollbacks some build.gradle refactoring
f6b4469 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka
fixes test
5492447 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka OlegDokuka force-pushed the bugfix/track-discarded branch 5 times, most recently from 75445c1 to 8a3103e Compare April 21, 2020 19:37
@OlegDokuka
fixes test
edf6788 
Signed-off-by: Oleh Dokuka <shadowgun@i.ua>
@OlegDokuka OlegDokuka force-pushed the bugfix/track-discarded branch from 8a3103e to edf6788 Compare April 21, 2020 19:44
@OlegDokuka OlegDokuka merged commit 070cffe into develop Apr 21, 2020
@OlegDokuka OlegDokuka deleted the bugfix/track-discarded branch April 21, 2020 20:12
@rstoyanchev rstoyanchev changed the title Track Discarded Payloads Use doOnDiscard to release cached Payloads Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonbasle simonbasle Awaiting requested review from simonbasle

@rstoyanchev rstoyanchev Awaiting requested review from rstoyanchev

Assignees

@OlegDokuka OlegDokuka

Labels
bug
Projects
None yet
Milestone
1.0 RC7
Development

Successfully merging this pull request may close these issues.
2 participants
@OlegDokuka @rstoyanchev