Re: [PR] [fix][broker] Handle synchronous schema lookup failures in replication [pulsar]

["Denovo1998 (via GitHub)" <gi...@apache.org>]

Denovo1998 commented on code in PR #26108:
URL: https://github.com/apache/pulsar/pull/26108#discussion_r3703939248


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentReplicator.java:
##########
@@ -455,19 +456,27 @@ public void readEntriesComplete(List<Entry> entries, Object ctx) {
                     .attr("atLeastOneMessageSentForReplication", atLeastOneMessageSentForReplication)
                     .attr("isWritable", isWritable())
                     .log("Pausing replication traffic");
+        } else if (waitForCursorRewindingRefCnf > 0) {
+            log.debug()
+                    .attr("reason", reasonOfWaitForCursorRewinding)
+                    .log("Skipping read while waiting for cursor rewind");

Review Comment:
   @lhotari 
   That makes sense. Since `readMoreEntries()` already checks `waitForCursorRewindingRefCnf` under the `inFlightTasks` lock, the outer guard is redundant and reads the rewind state outside its intended synchronization boundary.
   
   I will remove the guard from `readEntriesComplete()` and update the regression test to verify that no cursor read is scheduled while waiting for a rewind, rather than asserting that `readMoreEntries()` is not invoked.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org