refactor: process upstream messages individually if push fails #483
Conversation
yashmehrotra
force-pushed
the
upstream-err-ind-procss
branch
from
e017e08
to
b341ae9
Compare
| // Process each event individually since upsteam.Push is idempotent | ||
| var failedIndividualEvents []api.Event | ||
| for _, e := range events { | ||
| failedIndividualEvents = append(failedIndividualEvents, t.Run(ctx, []api.Event{e})...) |
There was a problem hiding this comment.
I think for the sake of simplicity we could avoid the recursive call in here. It would probably work in similar way I assume.
There was a problem hiding this comment.
I now see that Run would otherwise receive a group of events, so t.Run makes sense here.
Maybe we need to do the same for failedEvents.
Or in general, any failed event should be handled individually. What do you think of modifying consumeEvents() so when it's fetching in batches, it's making sure it's not pulling any failed events. And if it is pulling any failed event it's doing that one at a time.
This way the consumers do not have to handle this in ProcessBatchFunc
There was a problem hiding this comment.
In all other consumers, although we process events in bulk, each event passed or fails individually in isolation. There is no need to handle failed events separately there
For upstream push, we are mixing N events into one function call, the entirety of which can fail leading to us not knowing which event was the culprit, this is why in that scenario, we are just reprocessing all again individually so that the other successful events don't get marked as failed due to the failed events
In other consumers, this is already handled in the implementation since events do not get mixed
No description provided.