Skip to content

fix(BufferedConsumer): don't resend delivered batches after a partial flush failure - #191

Open
ckarnell wants to merge 1 commit into
mixpanel:masterfrom
ckarnell:fix/buffered-consumer-partial-flush
Open

fix(BufferedConsumer): don't resend delivered batches after a partial flush failure#191
ckarnell wants to merge 1 commit into
mixpanel:masterfrom
ckarnell:fix/buffered-consumer-partial-flush

Conversation

@ckarnell

Copy link
Copy Markdown

BufferedConsumer._flush_endpoint only writes the trimmed buffer back after the loop finishes, so when a later batch fails the earlier ones stay in the buffer even though the server already took them. The next flush sends them a second time.

It needs more than max_size messages queued to bite, which is what you get after any failed flush, since send() keeps appending to a buffer it couldn't drain.

With max_size=2 and a network that drops the first and third attempts:

bc = BufferedConsumer(max_size=2)
for ev in ['"a"', '"b"', '"c"', '"d"']:
    try:
        bc.send("events", ev)
    except MixpanelException:
        pass
bc.flush()

The API receives ["a","b"], then ["a","b"] again, then ["c","d"]. Same events, twice. Moving the write inside the loop keeps anything already delivered out of the buffer.

The new test fails on master with ['"a"', '"b"', '"c"', '"d"'] == ['"c"', '"d"'] and passes with the change. Full suite is 57 passing.

#16 and #54 both touched this code, but they were about concurrent flushes from mixpanel-python-async and were closed as belonging in that wrapper. This one doesn't involve threads.

@ckarnell
ckarnell requested review from a team and tdumitrescu August 15, 2026 19:53
@greptile-apps

greptile-apps Bot commented Aug 15, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge.

The buffer is advanced only after each successful send, while a failed batch and all remaining events stay available for retry.

Important Files Changed

Filename Overview
mixpanel/init.py Moves buffer advancement into the successful-send loop, preventing delivered batches from being retried after a partial failure.
test_mixpanel.py Adds focused regression coverage confirming that only undelivered events remain buffered after a partial flush failure.

Reviews (1): Last reviewed commit: "fix(BufferedConsumer): persist drained b..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant