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

PendingAddOp keeps refCnt onthe toSend buffer longer than needed. #1063

Closed
jvrao opened this issue Jan 26, 2018 · 2 comments
Closed

PendingAddOp keeps refCnt onthe toSend buffer longer than needed. #1063

jvrao opened this issue Jan 26, 2018 · 2 comments

Comments

@jvrao
Copy link
Contributor

jvrao commented Jan 26, 2018

BUG REPORT

  1. Please describe the issue you observed:
    PendingAddOp:maybeRecycle()->recycle() keeps the buffer until writeComplete() is called
    for each bookie write. We need to keep this buffer only until it is successfully transferred by netty.
    In the current code, the write is retired only if disableEnsembleChangeFeature is enabled. Otherwise, there is no point in keeping this buffer around.

Netty direct buffers are too scarce in the heavy workload scenarios. Keeping these buffers around longer than needed negatively affects the throughput on the client.

@sijie
Copy link
Member

sijie commented Jan 26, 2018

@jvrao don't we have to keep the buffer around for retries? any idea that you are thinking of to improve here?

@jvrao
Copy link
Contributor Author

jvrao commented Jan 31, 2018

@sijie toSend buffer is not needed for retries as we discussed on slack.

@sijie sijie added this to the 4.7.0 milestone Feb 3, 2018
@sijie sijie closed this as completed in 9d09a9c Feb 3, 2018
sijie pushed a commit that referenced this issue Feb 3, 2018
Descriptions of the changes in this PR:

Current code keeps the toSend buffers until client receives
resonses from all wq bookies. From the senders perspective,
It is not required to keep the refcount on it at the PendingAddOp
level, as the ref is taken at bookie client level.

Keeping these buffers longer will increase the memory pressure
on the client.

Signed-off-by: Venkateswararao Jujjuri (JV) <vjujjurisalesforce.com>

Master Issue: #1091

Author: JV Jujjuri <vjujjuri@vjujjuri-ltm2.internal.salesforce.com>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1091 from jvrao/bookkeeper-1063, closes #1063

(cherry picked from commit 9d09a9c)
Signed-off-by: Sijie Guo <sijie@apache.org>
sijie added a commit to sijie/bookkeeper that referenced this issue Mar 5, 2018
sijie added a commit to sijie/bookkeeper that referenced this issue Mar 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants