Clients should not depend on any of this behaviour, since the broker is at the other end of an unreliable network connection. The semantic differences are kind of meaningless from a usability point, since flushing on close =/= published. What exactly does "graceful" convey here? Flush the buffer on the client end and hope it makes it to the server.
Is there a difference whether you flush(or process) pending messages or not? There is no guarantee that either case will ensure the message is published. The only way to ensure that messages are published is to wait for the ack. The correct model should be to wait for return on the blocking API, or wait for future completion of the async API, then handle any publish errors and then only close the producer. On Mon, Sep 27, 2021 at 8:50 PM Yunze Xu <y...@streamnative.io.invalid> wrote: > Hi all, > > Recently I found a PR (https://github.com/apache/pulsar/pull/12195 < > https://github.com/apache/pulsar/pull/12195>) that > modifies the existing semantics of producer close. There're already some > communications in this PR, but I think it's better to start a discussion > here > to let more know. > > The existing implementation of producer close is: > 1. Cancel all timers, including send and batch container > (`batchMessageContainer`). > 2. Complete all pending messages (`pendingMessages`) with > `AlreadyCloseException`. > > See `ProducerImpl#closeAsync` for details. > > But the JavaDoc of `Producer#closeAsync` is: > > > No more writes will be accepted from this producer. Waits until all > pending write request are persisted. > > Anyway, the document and implementation are inconsistent. But specifically, > we need to define the behavior for how to process `pendingMessages` and > `batchMessageContainer` when producer call `closeAsync`. > > 1. batchMessageContainer: contains the buffered single messages > (`Message<T>`). > 2. pendingMessages: all inflight messages (`OpSendMsg`) in network. > > IMO, from the JavaDoc, only `pendingMessages` should be processed and the > messages in `batchMessageContainer` should be discarded. > > Since other clients might have already implemented the similar semantics of > Java clients. If we changed the semantics now, the behaviors among > different > clients might be inconsistent. > > Should we add a configuration to support graceful close to follow the > docs? Or > just change the current behavior? > > Thanks, > Yunze
