This is an automated email from the ASF dual-hosted git repository. sijie pushed a commit to branch branch-4.6 in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/branch-4.6 by this push: new 955f7ac ISSUE #1063: Write keeps refCnt longer 955f7ac is described below commit 955f7aceaefa9df131447f6005f4a16a50bc7172 Author: JV Jujjuri <vjujj...@vjujjuri-ltm2.internal.salesforce.com> AuthorDate: Fri Feb 2 16:22:56 2018 -0800 ISSUE #1063: Write keeps refCnt longer 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 <vjujj...@vjujjuri-ltm2.internal.salesforce.com> Reviewers: Enrico Olivelli <eolive...@gmail.com>, Sijie Guo <si...@apache.org> This closes #1091 from jvrao/bookkeeper-1063, closes #1063 (cherry picked from commit 9d09a9c2a64b745271ef1c6dad9e5ab3ab3f2a5c) Signed-off-by: Sijie Guo <si...@apache.org> --- .../org/apache/bookkeeper/client/PendingAddOp.java | 43 ++++++++++++++-------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingAddOp.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingAddOp.java index 5bc2b54..297b32a 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingAddOp.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingAddOp.java @@ -395,29 +395,40 @@ class PendingAddOp extends SafeRunnable implements WriteCallback, TimerTask { this.recyclerHandle = recyclerHandle; } + private void maybeRecycle() { - // The reference to PendingAddOp can be held in 3 places - // - LedgerHandle#pendingAddOp - // This reference is released when the callback is run - // - The executor - // Released after safeRun finishes - // - BookieClient - // Holds a reference from the point the addEntry requests are - // sent. - // The object can only be recycled after all references are - // released, otherwise we could end up recycling twice and all - // joy that goes along with that. - if (hasRun && callbackTriggered && pendingWriteRequests == 0) { - recycle(); + /** + * We have opportunity to recycle two objects here. + * PendingAddOp#toSend and LedgerHandle#pendingAddOp + * + * A. LedgerHandle#pendingAddOp: This can be released after all 3 conditions are met. + * - After the callback is run + * - After safeRun finished by the executor + * - Write responses are returned from all bookies + * as BookieClient Holds a reference from the point the addEntry requests are sent. + * + * B. ByteBuf can be released after 2 of the conditions are met. + * - After the callback is run as we will not retry the write after callback + * - After safeRun finished by the executor + * BookieClient takes and releases on this buffer immediately after sending the data. + * + * The object can only be recycled after the above conditions are met + * otherwise we could end up recycling twice and all + * joy that goes along with that. + */ + if (hasRun && callbackTriggered) { + ReferenceCountUtil.release(toSend); + toSend = null; + } + if (toSend == null && pendingWriteRequests == 0) { + recyclePendAddOpObject(); } } - private void recycle() { + private void recyclePendAddOpObject() { entryId = LedgerHandle.INVALID_ENTRY_ID; currentLedgerLength = -1; - ReferenceCountUtil.release(toSend); payload = null; - toSend = null; cb = null; ctx = null; ackSet.recycle(); -- To stop receiving notification emails like this one, please contact si...@apache.org.