eolivelli commented on code in PR #15914:
URL: https://github.com/apache/pulsar/pull/15914#discussion_r983183760
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java:
##########
@@ -873,23 +891,101 @@ public void getInfoFailed(ManagedLedgerException
exception, Object ctx) {
}, ctx);
}
+ private CompletableFuture<Void> cleanupOffloaded(long ledgerId, UUID uuid,
ManagedLedgerConfig mlConfig,
+ Map<String, String>
offloadDriverMetadata, String cleanupReason, String name) {
+ log.info("[{}] Cleanup offload for ledgerId {} uuid {} because of the
reason {}.",
+ name, ledgerId, uuid.toString(), cleanupReason);
+ Map<String, String> metadataMap = new HashMap();
+ metadataMap.putAll(offloadDriverMetadata);
+ metadataMap.put("ManagedLedgerName", name);
+
+ return
Retries.run(Backoff.exponentialJittered(TimeUnit.SECONDS.toMillis(1),
+ TimeUnit.SECONDS.toHours(1)).limit(10),
+ Retries.NonFatalPredicate,
+ () -> mlConfig.getLedgerOffloader().deleteOffloaded(ledgerId,
uuid, metadataMap),
+ scheduledExecutor, name).whenComplete((ignored, exception) -> {
+ if (exception != null) {
+ log.warn("[{}] Error cleaning up offload for {}, (cleanup
reason: {})",
+ name, ledgerId, cleanupReason, exception);
+ }
+ });
+ }
+
private void deleteManagedLedgerData(BookKeeper bkc, String
managedLedgerName, ManagedLedgerInfo info,
- DeleteLedgerCallback callback, Object ctx) {
+
CompletableFuture<ManagedLedgerConfig> mlConfigFuture,
+ DeleteLedgerCallback callback, Object
ctx) {
+ final CompletableFuture<Map<Long,
MLDataFormats.ManagedLedgerInfo.LedgerInfo>>
+ ledgerInfosFuture = new CompletableFuture<>();
+ store.getManagedLedgerInfo(managedLedgerName, false, null,
Review Comment:
Unfortunately ManagedLedgerInfo is part of of the public REST API
for instance here we return it (JSON encoded) to the client
https://github.com/apache/pulsar/blob/957a16b341197d6734f170a5cdcdd0500133c771/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L1262
We should not add everything to it.
We have this bad problem in Pulsar that we aren't always aware of what is
leaking to the public APIs.
I prefer to keep the patch in this form.
And if we want to change ManagedLedgerInfo we can do it in a follow up work.
As this patch is fixing some kind of "bad problem" (because we are not
deleting data that should have been deleted, that has some legal impact in some
countries), this patch should be cherry-picked to active branches.
I won't add API changes in a patch that will be ported
--
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]
