[bookkeeper] branch master updated: ISSUE #1063: Write keeps refCnt longer

[si...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 9d09a9c  ISSUE #1063: Write keeps refCnt longer
9d09a9c is described below

commit 9d09a9c2a64b745271ef1c6dad9e5ab3ab3f2a5c
Author: JV Jujjuri <vj...@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 <vj...@vjujjuri-ltm2.internal.salesforce.com>
    
    Reviewers: Enrico Olivelli <eo...@gmail.com>, Sijie Guo <si...@apache.org>
    
    This closes #1091 from jvrao/bookkeeper-1063, closes #1063
---
 .../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 77f05c1..e386701 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
@@ -383,29 +383,40 @@ class PendingAddOp extends SafeRunnable implements WriteCallback {
         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
sijie@apache.org.