Closed ekerstens closed 2 years ago
@ekerstens Use stream.noack().noack_take(1,1):
instead of events():
as events is bugged.
@zerafachris can you share any issues or details about the issue with events()
?
Looks like about that too: https://github.com/faust-streaming/faust/pull/208
@ekerstens The issue with events is that upon restart, the offset of the event causing the failure gets skipped and does not process. For example, consider that you have the following:
Topic Source Payloads:
{customer_id: 1, details: "something_1"}
{customer_id: 2, details: "something_2"}
{customer_id: 3, details: "something_3"}
{customer_id: 4, details: "something_4"}
{customer_id: 5, details: "something_5"}
{customer_id: 6, details: "something_6"}
{customer_id: 7, details: "something_7"}
{customer_id: 8, details: "something_8"}
{customer_id: 9, details: "something_9"}
If these are sent to the following agent:
@app.agent(topic_source, sink=[topic_sink])
async def process(stream):
async for record in stream.noack().event():
if record.value.get('customer_id') == 5:
raise exception
yield record
await stream.ack(record)
The agent will stop processing at customer_id 5. If you comment out the if-exception block, the app should continue processing and there should be a total of 9 unique distant payloads in the sink topic. However, what happens is that payload for 'customer_id' == 5 will get ignored upon restart.
A workaround for this is to use noack_take(1, 1)
. The agent becomes:
@app.agent(topic_source, sink=[topic_sink])
async def process(stream):
async for record in stream.noack().noack_take(1, 1):
new_record = record[0]
if new_record.value.get('customer_id') == 5:
raise exception
yield new_record
await stream.ack(record[0])
@zerafachris is there a separate issue open for this? #312 is fixed by #313 and can be closed. I'd like to understand more about events
vs. noack_take
but am not sure it's relevant to this ticket.
@ekerstens I did not open a different issue for this. If you do open one, let me know and I can contribute to it.
Fixed by https://github.com/faust-streaming/faust/pull/313. @zerafachris I've separately created https://github.com/faust-streaming/faust/issues/315 to discuss events
vs noack_take
Any stream processors using stream.noack() should be aware of a potential change which is required to upgrade to faust-streaming==0.8.5 due to this fix. Previously, since unacked messages were eventually committed anyway if a message with a higher offset was acked, unacked messages could go unnoticed. However, as a side effect this means that if a processor fails to ack any event, then the processor will never commit an offset past that point. For any apps with a flow like
def process(stream):
async for event in stream.noack().events():
do_something(event)
event.ack()
the code has to change to something like
def process(stream):
async for event in stream.noack().events():
try:
do_something(event)
finally:
event.ack()
or
async for event in stream.events():
async with event:
do_something(event)
or some other similar pattern. Otherwise, in case of an exception during do_something the event would never be acked, in which case offsets will never progress past that point.
Checklist
master
branch of Faust.Steps to reproduce
Create an agent consuming from a topic defined like
Expected behavior
Faust should be able to identify a gap between the committed offset and the first acked message to avoid skipping messages. Per the documentation in consumer._new_offset
Since the above scenario, we never ack every 5th message, the offset should never continue as there is a gap.
Actual behavior
app.consumer._acked == [1,2,3,4,6,7]
Consumer._new_offset
is called for the first time, a gap is identified at 5 and so offset 5 is committed.app.consumer._acked == [6,7]
Consumer._new_offset
is called next, there is no gap, so the consumer commits offset 8.Versions
Traceback/Logs