You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/09/08 14:10:43 UTC
[GitHub] [pulsar] codelipenghui commented on a diff in pull request #17512: [fix][tiered-storage] Don't cleanup data when offload met Metastore exception
codelipenghui commented on code in PR #17512:
URL: https://github.com/apache/pulsar/pull/17512#discussion_r966019339
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -3004,8 +3004,19 @@ private void offloadLoop(CompletableFuture<PositionImpl> promise, Queue<LedgerIn
scheduledExecutor, name)
.whenComplete((ignore2, exception) -> {
if (exception != null) {
- log.error("[{}] Failed to offload data for the ledgerId {}",
+ Throwable e = FutureUtil.unwrapCompletionException(exception);
+ if (e instanceof MetaStoreException) {
+ // When a MetaStore exception happens, we can not make sure the metadata
+ // update is failed or not. Because we have a retry on the connection loss,
+ // it is possible to get a BadVersion or other exception after retrying.
+ // So we don't clean up the data if it has metadata operation exception.
+ log.error("[{}] Failed to update offloaded metadata for the ledgerId {}",
name, ledgerId, exception);
+ return;
+ } else {
+ log.error("[{}] Failed to offload data for the ledgerId {}",
+ name, ledgerId, exception);
Review Comment:
`Failed to offload data for the ledgerId, try to clean up the offloaded data`
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -3004,8 +3004,19 @@ private void offloadLoop(CompletableFuture<PositionImpl> promise, Queue<LedgerIn
scheduledExecutor, name)
.whenComplete((ignore2, exception) -> {
if (exception != null) {
- log.error("[{}] Failed to offload data for the ledgerId {}",
+ Throwable e = FutureUtil.unwrapCompletionException(exception);
+ if (e instanceof MetaStoreException) {
+ // When a MetaStore exception happens, we can not make sure the metadata
+ // update is failed or not. Because we have a retry on the connection loss,
+ // it is possible to get a BadVersion or other exception after retrying.
+ // So we don't clean up the data if it has metadata operation exception.
+ log.error("[{}] Failed to update offloaded metadata for the ledgerId {}",
Review Comment:
I think it's better to add more information like
`Failed to update offloaded metadata for the ledgerId, the offloaded data will not be cleaned up`
--
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