[GitHub] [pulsar] codelipenghui commented on a diff in pull request #21010: [fix][meta] Fix deadlock in AutoRecovery.

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

codelipenghui commented on code in PR #21010:
URL: https://github.com/apache/pulsar/pull/21010#discussion_r1299497597


##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/bookkeeper/PulsarLedgerUnderreplicationManager.java:
##########
@@ -978,11 +978,13 @@ public long getReplicasCheckCTime() throws ReplicationException.UnavailableExcep
     @Override
     public void notifyUnderReplicationLedgerChanged(BookkeeperInternalCallbacks.GenericCallback<Void> cb)
             throws ReplicationException.UnavailableException {
-        log.debug("notifyUnderReplicationLedgerChanged()");
-        store.registerListener(e -> {
-            if (e.getType() == NotificationType.Deleted && ID_EXTRACTION_PATTERN.matcher(e.getPath()).find()) {
-                cb.operationComplete(0, null);
-            }
-        });
+        //The store listener callback executor is metadata-store executor,
+        //in cb.operationComplete(0, null), it will get all underreplication ledgers from metadata-store, it's sync
+        //operation. So it's a deadlock.
+//        store.registerListener(e -> {
+//            if (e.getType() == NotificationType.Deleted && ID_EXTRACTION_PATTERN.matcher(e.getPath()).find()) {
+//                cb.operationComplete(0, null);
+//            }
+//        });

Review Comment:
   Why not remove these lines and add an explanation of why we should keep the method empty.



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