michaeljmarshall commented on code in PR #11592:
URL: https://github.com/apache/pulsar/pull/11592#discussion_r1143953299
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java:
##########
@@ -458,10 +458,10 @@ public void markDeleteComplete(Object ctx) {
@Override
public void markDeleteFailed(ManagedLedgerException exception, Object
ctx) {
- // TODO: cut consumer connection on markDeleteFailed
if (log.isDebugEnabled()) {
log.debug("[{}][{}] Failed to mark delete for position {}:
{}", topicName, subName, ctx, exception);
}
+ dispatcher.getConsumers().forEach(Consumer::disconnect);
Review Comment:
I see that we have a comment to cut the consumer connection, but this
comment is from the initial import, so I'd like to make sure that it is still
the right behavior. Note that we've operated for many years without
disconnecting these consumers.
This `markDeleteCallback` object is only used for cumulative acknowledgement
here:
https://github.com/apache/pulsar/blob/cdeef00c5f6a5bd3197b4ca6de0a0505b18835d8/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java#L392-L393
In that case, failure means the mark delete position wasn't moved forward.
Do we understand what would cause failure here? I assume it could come from
bookies that are full or removed, network issues, or general bookie issues.
Closing the connection for each consumer will lead them to reconnect and
attempt to fetch unacknowledged messages. In the case of cumulative
acknowledgement, the consumer will fetch the messages that it just tried to
ack. This is probably better than progressing indefinitely, but given that this
hasn't appeared to be an issue, I am wondering if we really need this change.
(I am open to the change, just want to discuss it a bit more. Thanks!)
--
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]
