apurtell commented on code in PR #4613:
URL: https://github.com/apache/hbase/pull/4613#discussion_r918457818


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/BufferChain.java:
##########
@@ -61,51 +60,27 @@ boolean hasRemaining() {
     return remaining > 0;
   }
 
-  /**
-   * Write out our chain of buffers in chunks
-   * @param channel   Where to write
-   * @param chunkSize Size of chunks to write.
-   * @return Amount written. n
-   */
-  long write(GatheringByteChannel channel, int chunkSize) throws IOException {
-    int chunkRemaining = chunkSize;
-    ByteBuffer lastBuffer = null;
-    int bufCount = 0;
-    int restoreLimit = -1;
-
-    while (chunkRemaining > 0 && bufferOffset + bufCount < buffers.length) {
-      lastBuffer = buffers[bufferOffset + bufCount];
-      if (!lastBuffer.hasRemaining()) {
-        bufferOffset++;
-        continue;
-      }
-      bufCount++;
-      if (lastBuffer.remaining() > chunkRemaining) {
-        restoreLimit = lastBuffer.limit();
-        lastBuffer.limit(lastBuffer.position() + chunkRemaining);
-        chunkRemaining = 0;
-        break;
-      } else {
-        chunkRemaining -= lastBuffer.remaining();
-      }
-    }
-    assert lastBuffer != null;
-    if (chunkRemaining == chunkSize) {
-      assert !hasRemaining();
-      // no data left to write
+  long write(GatheringByteChannel channel) throws IOException {
+    if (!hasRemaining()) {
       return 0;
     }
-    try {
-      long ret = channel.write(buffers, bufferOffset, bufCount);
-      if (ret > 0) {
-        remaining = (int) (remaining - ret);
-      }
-      return ret;
-    } finally {
-      if (restoreLimit >= 0) {
-        lastBuffer.limit(restoreLimit);
+    long written = 0;
+    for (ByteBuffer bb : this.buffers) {
+      if (bb.hasRemaining()) {
+        final int pos = bb.position();
+        final int result = channel.write(bb);
+        if (result <= 0) {
+          // Write error. Return how much we were able to write until now.
+          return written;
+        }
+        // Adjust the position of buffers already written so we don't write out
+        // duplicate data upon retry of incomplete write with the same buffer 
chain.
+        bb.position(pos + result);

Review Comment:
   I don't think we will hit this case. If the write for a call response fails 
an exception will be raised and that's it for that particular call and 
response. If the client decides to retry there will be a new call, and a new 
response, and a new write with a new bufferchain. That said I wanted this code 
to be correct if it happens, perhaps after some future change.



-- 
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.

To unsubscribe, e-mail: [email protected]

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

Reply via email to