[DLOG] LogReader shouldn't be added to pending if not locking#2185
Merged
ivankelly merged 1 commit intoapache:masterfrom Oct 23, 2019
Merged
[DLOG] LogReader shouldn't be added to pending if not locking#2185ivankelly merged 1 commit intoapache:masterfrom
ivankelly merged 1 commit intoapache:masterfrom
Conversation
The pendingReaders set in BKDistributedLogManager exists so that if the manager is closed which the lock is being acquired for a reader, that reader will be closed (even though it hasn't been returned to the client). In the case that the reader is opened without a lock, there is not async action being performed. Previously we were also adding these readers to the pendingReaders, but they were never being removed from the pendingReaders, causing a memory leak. This change avoids adding no-locking readers to pendingReaders.
Contributor
|
This code is new to me, but the change looks good to me. Unfortunately ASF CI is is bad state. |
eolivelli
approved these changes
Oct 21, 2019
Contributor
|
rebuild java8 |
Contributor
|
rebuild java8 |
Contributor
|
run bookkeeper-server bookie tests |
ivankelly
added a commit
that referenced
this pull request
Oct 23, 2019
The pendingReaders set in BKDistributedLogManager exists so that if the manager is closed which the lock is being acquired for a reader, that reader will be closed (even though it hasn't been returned to the client). In the case that the reader is opened without a lock, there is not async action being performed. Previously we were also adding these readers to the pendingReaders, but they were never being removed from the pendingReaders, causing a memory leak. This change avoids adding no-locking readers to pendingReaders. Reviewers: Enrico Olivelli <eolivelli@gmail.com> This closes #2185 from ivankelly/orphan-reader (cherry picked from commit 4f56edf) Signed-off-by: Ivan Kelly <ivank@apache.org>
ivankelly
added a commit
that referenced
this pull request
Oct 23, 2019
The pendingReaders set in BKDistributedLogManager exists so that if the manager is closed which the lock is being acquired for a reader, that reader will be closed (even though it hasn't been returned to the client). In the case that the reader is opened without a lock, there is not async action being performed. Previously we were also adding these readers to the pendingReaders, but they were never being removed from the pendingReaders, causing a memory leak. This change avoids adding no-locking readers to pendingReaders. Reviewers: Enrico Olivelli <eolivelli@gmail.com> This closes #2185 from ivankelly/orphan-reader (cherry picked from commit 4f56edf) Signed-off-by: Ivan Kelly <ivank@apache.org>
zymap
pushed a commit
to streamnative/bookkeeper-achieved
that referenced
this pull request
May 15, 2020
The pendingReaders set in BKDistributedLogManager exists so that if the manager is closed which the lock is being acquired for a reader, that reader will be closed (even though it hasn't been returned to the client). In the case that the reader is opened without a lock, there is not async action being performed. Previously we were also adding these readers to the pendingReaders, but they were never being removed from the pendingReaders, causing a memory leak. This change avoids adding no-locking readers to pendingReaders. Reviewers: Enrico Olivelli <eolivelli@gmail.com> This closes apache#2185 from ivankelly/orphan-reader
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The pendingReaders set in BKDistributedLogManager exists so that if
the manager is closed which the lock is being acquired for a reader,
that reader will be closed (even though it hasn't been returned to the
client).
In the case that the reader is opened without a lock, there
is not async action being performed. Previously we were also adding
these readers to the pendingReaders, but they were never being removed
from the pendingReaders, causing a memory leak. This change avoids
adding no-locking readers to pendingReaders.