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 2021/09/20 16:36:32 UTC
[GitHub] [pulsar] merlimat opened a new pull request #12101: ManagedLedger should not attempt deferrable metadata operation while disconnected
merlimat opened a new pull request #12101:
URL: https://github.com/apache/pulsar/pull/12101
### Motivation
When we detect that we don't have a working connection or session to the metadata service, we need to avoid doing any operation that will involve access to the metadata store, if such operation can be deferred to later on.
The reason is that all these operations are obviously going to fail if there's session with metadata, while the current data path could be working perfectly fine. So, we want to keep the data path untouched, deferring all the ledger rollovers, until the situation with metadata store service clears 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
[GitHub] [pulsar] merlimat commented on pull request #12101: ManagedLedger should not attempt deferrable metadata operation while disconnected
Posted by GitBox <gi...@apache.org>.
merlimat commented on pull request #12101:
URL: https://github.com/apache/pulsar/pull/12101#issuecomment-923274901
> Possibly a version of this patch would be very useful for branch-2.8
This will not fix the root cause of the other issue, as it there's not a way to precisely prevent requests while disconnected (eg: we could get disconnected while the request was already issued).
Rather, this is trying to prevent the majority of operations from trying, so it will definitely reduce the likelihood of that happening.
--
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
[GitHub] [pulsar] dlg99 commented on pull request #12101: ManagedLedger should not attempt deferrable metadata operation while disconnected
Posted by GitBox <gi...@apache.org>.
dlg99 commented on pull request #12101:
URL: https://github.com/apache/pulsar/pull/12101#issuecomment-923168157
@eolivelli FYI. I think this could be the real rootcause fix for https://github.com/apache/pulsar/issues/8677 and https://github.com/apache/pulsar/issues/12070 where the managed ledger's metadata got out of sync with the data in BK.
--
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
[GitHub] [pulsar] dlg99 commented on pull request #12101: ManagedLedger should not attempt deferrable metadata operation while disconnected
Posted by GitBox <gi...@apache.org>.
dlg99 commented on pull request #12101:
URL: https://github.com/apache/pulsar/pull/12101#issuecomment-923346309
> This will not fix the root cause of the other issue, as it there's not a way to precisely prevent requests while disconnected (eg: we could get disconnected while the request was already issued).
@merlimat I do agree with this assessment.
Do we really need to prevent requests when disconnected?
I think we can split managed ledger trim into two phases:
1. update metadata to include active BK ledgers (what we have now) + list of BK ledgers to delete. Do it as a single metadata update, in one transaction.
2. process list of BK ledgers to delete, actually delete the ledgers (skip if they were deleted already), possibly update metadata (or defer the update until next trim or managed ledger init)
I think this will prevent managed ledger from getting metadata out of sync with the data as result of trim/
--
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
[GitHub] [pulsar] merlimat merged pull request #12101: ManagedLedger should not attempt deferrable metadata operation while disconnected
Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #12101:
URL: https://github.com/apache/pulsar/pull/12101
--
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
[GitHub] [pulsar] merlimat commented on a change in pull request #12101: ManagedLedger should not attempt deferrable metadata operation while disconnected
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #12101:
URL: https://github.com/apache/pulsar/pull/12101#discussion_r712494496
##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java
##########
@@ -207,6 +218,22 @@ public BookKeeper get(EnsemblePlacementPolicyConfig policy) {
}
}
+ private synchronized void handleMetadataStoreNotification(SessionEvent e) {
+ log.info("Received MetadataStore session event: {}", e);
+
+ switch (e) {
Review comment:
I'm not sure where you're proposing to move this to
--
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
[GitHub] [pulsar] merlimat commented on pull request #12101: ManagedLedger should not attempt deferrable metadata operation while disconnected
Posted by GitBox <gi...@apache.org>.
merlimat commented on pull request #12101:
URL: https://github.com/apache/pulsar/pull/12101#issuecomment-923443180
@dlg99 The purpose of this change is not to avoid the out-of-sync situation. This is to ensure, as a best effort, that when we're losing connectivity with ZK we don't interrupt the data path.
Right now, since we're killing the broker anyway, it's not a big of a change, but this makes more sense when we're not going to kill the brokers on ZK session expired.
--
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
[GitHub] [pulsar] lhotari commented on pull request #12101: ManagedLedger should not attempt deferrable metadata operation while disconnected
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #12101:
URL: https://github.com/apache/pulsar/pull/12101#issuecomment-923705925
@merlimat Would it make sense to backport this change to branch-2.8 / branch-2.7?
--
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
[GitHub] [pulsar] eolivelli commented on a change in pull request #12101: ManagedLedger should not attempt deferrable metadata operation while disconnected
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #12101:
URL: https://github.com/apache/pulsar/pull/12101#discussion_r712358254
##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java
##########
@@ -207,6 +218,22 @@ public BookKeeper get(EnsemblePlacementPolicyConfig policy) {
}
}
+ private synchronized void handleMetadataStoreNotification(SessionEvent e) {
+ log.info("Received MetadataStore session event: {}", e);
+
+ switch (e) {
Review comment:
Is it better to move 'switch' to some utility method?
--
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