ivankelly commented on a change in pull request #720: Fixed simultaneus reads 
on same ledger/entry with v2 protocol
URL: https://github.com/apache/bookkeeper/pull/720#discussion_r150482316
 
 

 ##########
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
 ##########
 @@ -732,14 +729,18 @@ private void readEntryInternal(final long ledgerId,
                     .build();
         }
 
-        CompletionValue completion = new ReadCompletion(completionKey, cb,
-                                                        ctx, ledgerId, 
entryId);
-        CompletionValue existingValue = completionObjects.putIfAbsent(
-                completionKey, completion);
+        ReadCompletion readCompletion = new ReadCompletion(completionKey, cb, 
ctx, ledgerId, entryId);
+        CompletionValue existingValue = 
completionObjects.putIfAbsent(completionKey, readCompletion);
         if (existingValue != null) {
-            // There's a pending read request on same ledger/entry. This is 
not supported in V2 protocol
-            LOG.warn("Failing concurrent request to read at ledger: {} entry: 
{}", ledgerId, entryId);
-            completion.errorOut(BKException.Code.UnexpectedConditionException);
+            // There's a pending read request on same ledger/entry. Use the 
multimap to track all of them
+            synchronized (this) {
+                completionObjectsV2Conflicts.put(completionKey, 
readCompletion);
+            }
+        }
+
+        final Channel c = channel;
 
 Review comment:
   This is handled in writeAndFlush now. You'll need to add a check for the 
conflicts map into the #errorOut method.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to