Decide on how to use poll and flush in producer

[timmc-edx]

We currently use poll() on the producer, but it wasn't clear whether we should use that, or how (or with what timeout), and when we should use flush() instead/too.

event-bus-kafka/edx_event_bus_kafka/publishing/event_producer.py

Line 221 in f0f7386

producer.poll() # wait indefinitely for the above event to either be delivered or fail

Also investigate:

• enable.idempotence=true
• acks=all

Resources:

• Kafka documentation, very thorough overall: https://kafka.apache.org/documentation/
• librdkafka (that's the implementation we're using, indirectly) FAQ on producer poll: https://github.com/edenhill/librdkafka/wiki/FAQ#when-and-how-should-i-call-rd_kafka_poll

[timmc-edx]

OK, this is my new understanding of librdkafka producers:

• Messages we produce will actually be accumulated into batches before (asynchronous) delivery to the broker, with batch size and timing depending on our producer settings. Batching is fundamental to Kafka's performance.
• Incoming acks to those messages accumulate asynchronously in a buffer, and do not immediately trigger callbacks.
• The only job of poll() is to wait for acks and trigger callbacks—it doesn't affect delivery to the broker at all. poll(0) checks if anything is already in that buffer, and triggers callbacks to be made if there were any. Passing a non-zero timeout makes poll() first block until there's at least one message.
  • Question: Are callbacks made on the same thread as poll()?
• Something needs to call poll() regularly to ensure callbacks are triggered.
• flush() both ensures that the outgoing message batch is sent and that all acks have been received (and callbacks triggered, with them).

Implications:

• When manually testing, the produce-event management command needs to call flush() before returning, otherwise the pending batch containing the outgoing message will be lost. Or, potentially, we may need to change the batch delivery settings when doing a manual test so that delivery is immediate—but that still gives a potential race condition.
  • Addressed in fix: Use non-blocking poll after production; allow flush for testing #12
• In our real event-producing code, we should call poll(0) every time we send an event, either before or after, just to make sure callbacks are getting triggered and the ack buffer is getting drained. If event production is very infrequent, and we care about callbacks being timely, we need a separate thread calling poll() with a timeout.
  • Addressed in fix: Use non-blocking poll after production; allow flush for testing #12
• We need to call flush() on server shutdown, somehow
  • Deferred to later epic: [Producer] Make sure we don't lose events on producer shutdown #11

[timmc-edx]

enable.idempotence=true and acks=all turn out to be the default values, and are what we want.

[mparada-suva]

[...] Passing a non-zero timeout makes poll() first block until there's at least one message.

Hi! Do you know some reference to this point you mentioned? We are seeing such a problem and would like to investigate how to mitigate it.

[mparada-suva]

Maybe I found it... Would this be the ref: https://docs.confluent.io/platform/7.5/clients/librdkafka/html/classRdKafka_1_1Handle.html#a98d3b4ee48457ff13e4d5155e3fc5ea4 ?

[timmc-edx]

Yep, that looks right. You can see what we ended up doing, along with the reasoning (note that we call poll from two places.)