sijie commented on a change in pull request #8244:
URL: https://github.com/apache/pulsar/pull/8244#discussion_r506572994
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataTeardown.java
##########
@@ -107,5 +128,86 @@ public static void deleteZkNodeRecursively(ZooKeeper
zooKeeper, String path) thr
}
}
+ private static List<String> getChildren(ZooKeeper zooKeeper, String path) {
+ try {
+ return zooKeeper.getChildren(path, null);
+ } catch (InterruptedException | KeeperException e) {
+ log.error("Failed to get children of {}: {}", path, e);
+ throw new RuntimeException(e);
+ }
+ }
+
+ private static byte[] getData(ZooKeeper zooKeeper, String path) {
+ try {
+ return zooKeeper.getData(path, null, null);
+ } catch (KeeperException | InterruptedException e) {
+ log.error("Failed to get data from {}: {}", path, e);
+ throw new RuntimeException(e);
+ }
+ }
+
+ private static void deleteLedger(BookKeeper bookKeeper, long ledgerId) {
+ try {
+ bookKeeper.deleteLedger(ledgerId);
+ log.debug("Delete ledger id: {}", ledgerId);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ } catch (BKException e) {
+ log.warn("Failed to delete ledger {}: {}", ledgerId, e);
+ }
+ }
+
+ private static void deleteManagedLedgers(ZooKeeper zooKeeper, BookKeeper
bookKeeper) {
+ final String managedLedgersRoot = "/managed-ledgers";
+ getChildren(zooKeeper, managedLedgersRoot).forEach(tenant -> {
+ final String tenantRoot = managedLedgersRoot + "/" + tenant;
+ getChildren(zooKeeper, tenantRoot).forEach(namespace -> {
+ final String namespaceRoot = String.join("/", tenantRoot,
namespace, "persistent");
+ getChildren(zooKeeper, namespaceRoot).forEach(topic -> {
Review comment:
I think the logic here can be replaced by using
`ManagedLedgerFactory#delete` to delete the ledger, no?
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataTeardown.java
##########
@@ -107,5 +128,86 @@ public static void deleteZkNodeRecursively(ZooKeeper
zooKeeper, String path) thr
}
}
+ private static List<String> getChildren(ZooKeeper zooKeeper, String path) {
+ try {
+ return zooKeeper.getChildren(path, null);
+ } catch (InterruptedException | KeeperException e) {
+ log.error("Failed to get children of {}: {}", path, e);
+ throw new RuntimeException(e);
+ }
+ }
+
+ private static byte[] getData(ZooKeeper zooKeeper, String path) {
+ try {
+ return zooKeeper.getData(path, null, null);
+ } catch (KeeperException | InterruptedException e) {
+ log.error("Failed to get data from {}: {}", path, e);
+ throw new RuntimeException(e);
+ }
+ }
+
+ private static void deleteLedger(BookKeeper bookKeeper, long ledgerId) {
+ try {
+ bookKeeper.deleteLedger(ledgerId);
+ log.debug("Delete ledger id: {}", ledgerId);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ } catch (BKException e) {
+ log.warn("Failed to delete ledger {}: {}", ledgerId, e);
+ }
+ }
+
+ private static void deleteManagedLedgers(ZooKeeper zooKeeper, BookKeeper
bookKeeper) {
+ final String managedLedgersRoot = "/managed-ledgers";
+ getChildren(zooKeeper, managedLedgersRoot).forEach(tenant -> {
+ final String tenantRoot = managedLedgersRoot + "/" + tenant;
+ getChildren(zooKeeper, tenantRoot).forEach(namespace -> {
+ final String namespaceRoot = String.join("/", tenantRoot,
namespace, "persistent");
+ getChildren(zooKeeper, namespaceRoot).forEach(topic -> {
+ final String topicRoot = namespaceRoot + "/" + topic;
+ byte[] topicData = getData(zooKeeper, topicRoot);
+ try {
+
ManagedLedgerInfo.parseFrom(topicData).getLedgerInfoList().stream()
+ .map(ManagedLedgerInfo.LedgerInfo::getLedgerId)
+ .forEach(ledgerId -> deleteLedger(bookKeeper,
ledgerId));
+
+ getChildren(zooKeeper,
topicRoot).stream().map(subscription -> {
+ final String subscriptionRoot = topicRoot + "/" +
subscription;
+ try {
+ return
ManagedCursorInfo.parseFrom(getData(zooKeeper,
subscriptionRoot)).getCursorsLedgerId();
+ } catch (InvalidProtocolBufferException e) {
+ log.warn("Invalid data format from {}: {}",
subscriptionRoot, e);
+ return -1L;
Review comment:
I don't think you should return `-1L` here. This will trigger
`deleteLedger(bookkeeper, -1L)` in line 183. This is not a good implementation.
You should return `null` and skip deletion if it is `null`.
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataTeardown.java
##########
@@ -107,5 +128,86 @@ public static void deleteZkNodeRecursively(ZooKeeper
zooKeeper, String path) thr
}
}
+ private static List<String> getChildren(ZooKeeper zooKeeper, String path) {
+ try {
+ return zooKeeper.getChildren(path, null);
+ } catch (InterruptedException | KeeperException e) {
+ log.error("Failed to get children of {}: {}", path, e);
+ throw new RuntimeException(e);
+ }
+ }
+
+ private static byte[] getData(ZooKeeper zooKeeper, String path) {
+ try {
+ return zooKeeper.getData(path, null, null);
+ } catch (KeeperException | InterruptedException e) {
+ log.error("Failed to get data from {}: {}", path, e);
+ throw new RuntimeException(e);
+ }
+ }
+
+ private static void deleteLedger(BookKeeper bookKeeper, long ledgerId) {
+ try {
+ bookKeeper.deleteLedger(ledgerId);
+ log.debug("Delete ledger id: {}", ledgerId);
Review comment:
we wrap `log.debug` in `if (log.isDebugEnabled())`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
