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

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

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


##########
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:
   I want the user to know there may be a deadlock when they see this code.  I agree with you that we should left the comments for user to let them know that PulsarLedgerUnderreplicationManager#notifyUnderReplicationLedgerChanged will be removed at next bk major release. But it's already merged, so if user confused, they could see this pr link to get the details.



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