merlimat commented on PR #22841: URL: https://github.com/apache/pulsar/pull/22841#issuecomment-2150448235
>umm.. which interface? The issue is the interface in BookKeeper client which doesn't support async call and I don't recall any such interface in BookKeeper introduced by me. `BookkeeperFactoryForCustomEnsemblePlacementPolicy` Sure, the main issue is in BK client, though this issue is only present when using different BK clients based on namespace policy. This is an example of problematic code, try/catch masking a blocking operation: https://github.com/apache/pulsar/blob/342d88dd193bb85c0af91c5193b1422808a9c821/pulsar-broker/src/main/java/org/apache/pulsar/broker/ManagedLedgerClientFactory.java#L100-L109 That also breaks the strict bookie assignment requirement that you were mentioning, because it will fallback to default BK client if there is any failure to read the bookie mapping from metadata. What's worse, it will not self-correct, and it will continue to use the wrong BK client indefinitely. ```java // find or create bk-client in cache for a specific ensemblePlacementPolicy if (ensemblePlacementPolicyConfig != null && ensemblePlacementPolicyConfig.getPolicyClass() != null) { bkClient = bkEnsemblePolicyToBkClientMap.computeIfAbsent(ensemblePlacementPolicyConfig, (key) -> { try { return bookkeeperProvider.create(conf, metadataStore, eventLoopGroup, Optional.ofNullable(ensemblePlacementPolicyConfig.getPolicyClass()), ensemblePlacementPolicyConfig.getProperties(), statsLogger); } catch (Exception e) { log.error("Failed to initialize bk-client for policy {}, properties {}", ensemblePlacementPolicyConfig.getPolicyClass(), ensemblePlacementPolicyConfig.getProperties(), e); } return this.defaultBkClient; }); } ``` -- 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]
