BewareMyPower edited a comment on pull request #10330:
URL: https://github.com/apache/pulsar/pull/10330#issuecomment-825875852


   @dlg99 I just found there's something wrong with @codelipenghui 's analysis, 
which leads to a result that your PR didn't work.
   
   We can debug `BrokerEntryMetadataE2ETest` and go to 
https://github.com/apache/pulsar/blob/5a7ba52193c069bc40705bcdc74287aae1066b17/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java#L123-L128
   
   - The original `data` is `io.netty.buffer.CompositeByteBuf`
   - However, `duplicateBuffer` is `io.netty.buffer.UnpooledDuplicatedByteBuf`
   
   So the argument type, which is passed to `LedgerHandle#asyncAddEntry`, is 
`UnpooledDuplicatedByteBuf` not `CompositeByteBuf`.
   
   ----
   
   I tried to change the code above to
   
   ```java
               ByteBuf duplicateBuffer = data.retainedDuplicate();
               final ByteBuf originalData = data;
               data = duplicateBuffer;
   
               // internally asyncAddEntry() will take the ownership of the 
buffer and release it at the end
               addOpCount = 
ManagedLedgerImpl.ADD_OP_COUNT_UPDATER.incrementAndGet(ml);
               lastInitTime = System.nanoTime();
               ledger.asyncAddEntry(originalData, this, addOpCount);
   ```
   
   But it still didn't work (even worse). I've added some debug logs to BK:
   
   ```java
       public static int resumeChecksum(int previousChecksum, ByteBuf payload) {
           log.info("XYZ resumeChecksum payload.hasMemoryAddress(): {}, 
hasArray(): {}, is CompositeByteBuf: {},"
                           + " class name: {}",
                   payload.hasMemoryAddress(), payload.hasArray(), (payload 
instanceof CompositeByteBuf),
                   payload.getClass().getName());
   ```
   
   Then send 1 message, the logs is:
   
   > 19:41:21.303 [BookKeeperClientWorker-OrderedExecutor-12-0] INFO  
com.scurrilous.circe.checksum.Crc32cIntChecksum - XYZ resumeChecksum 
payload.hasMemoryAddress(): true, hasArray(): false, is CompositeByteBuf: 
false, class name: io.netty.buffer.PooledUnsafeDirectByteBuf
   19:41:21.304 [BookKeeperClientWorker-OrderedExecutor-12-0] INFO  
com.scurrilous.circe.checksum.Crc32cIntChecksum - XYZ resumeChecksum 
payload.hasMemoryAddress(): true, hasArray(): false, is CompositeByteBuf: 
false, class name: io.netty.buffer.PooledUnsafeDirectByteBuf
   19:41:21.305 [BookKeeperClientWorker-OrderedExecutor-12-0] INFO  
com.scurrilous.circe.checksum.Crc32cIntChecksum - XYZ resumeChecksum 
payload.hasMemoryAddress(): true, hasArray(): false, is CompositeByteBuf: 
false, class name: 
io.netty.buffer.AbstractPooledDerivedByteBuf$PooledNonRetainedSlicedByteBuf
   
   The `CompositeByteBuf` was split to a `PooledUnsafeDirectByteBuf` and a 
`AbstractPooledDerivedByteBuf$PooledNonRetainedSlicedByteBuf`. It seems that 
`nioBuffer()` wouldn't be called, but the heap memory problem still existed and 
might get worse.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to