merlimat commented on a change in pull request #1171: Fixed managed ledger 
missing callback issue when unloading a topic
URL: https://github.com/apache/incubator-pulsar/pull/1171#discussion_r165869030
 
 

 ##########
 File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/Producer.java
 ##########
 @@ -279,8 +280,12 @@ public void completed(Exception exception, long ledgerId, 
long entryId) {
                         ? ServerError.TopicTerminatedError : 
ServerError.PersistenceError;
 
                 producer.cnx.ctx().channel().eventLoop().execute(() -> {
-                    
producer.cnx.ctx().writeAndFlush(Commands.newSendError(producer.producerId, 
sequenceId, serverError,
-                            exception.getMessage()));
+                    if (!(exception instanceof TopicClosedException)) {
 
 Review comment:
   > Here, broker may have sent close producer response but what is the 
downside here if we still send a response back?
   
   There's no need to send a bunch of `PersistenceError` back to the client in 
this case. Also the current reaction of the client when receiving these errors 
is to close connection and reconnect which will be overkill here.
   
   > I think in old client, it didn't support close-producer command so, in 
that case shouldn't we send this response back?
   
   If we saw the client didn't support the `CloseProducer`  command, we would 
have anyway closed the connection, so even in this case we don't need to 
propagate back the send-errors.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to