Avoid reordering events due to split partition queues #2437
Conversation
Great care is required when splitting queues to avoid losing ordering. We rely on ordering for dedupe so reordering -> dropped kafka messages.
|
We also need this on main I guess |
|
ill base on main and we can cherry pick later |
| }; | ||
| for task_id in topic_partition_tasks.into_values() { | ||
| self.task_center.cancel_task(task_id); | ||
| Rebalance::Error(_) => {} |
There was a problem hiding this comment.
This gets propagated in the main loop i suppose?
There was a problem hiding this comment.
no, it doesnt get propagated, but also from my reading of librdkafka it cant happen, and actually their code generally assumes that it doesnt happen and has weird behaviour if it does. however, i can see that rust-rdkafka treats this scenario as equivalent to revoke, so i guess i can do the same
There was a problem hiding this comment.
actually i cant treat it as equivalent to revoke, as they dont give me a handle on what the provided partitions are. my feeling is we can either ignore or panic
There was a problem hiding this comment.
Let's panic only if there's a panic handler somewhere that makes sure the panic won't get propagated and tear down the whole node. I think it should be the case with task center/subscription controller?
There was a problem hiding this comment.
imo its better to just ignore it
| @@ -418,13 +412,11 @@ impl ConsumerContext for RebalanceContext { | |||
| } | |||
| } | |||
|
|
|||
| struct AbortOnDrop(TaskCenter, TaskId); | |||
| struct AbortOnDrop(TaskHandle<()>); | |||
There was a problem hiding this comment.
Optional: This is a handy type if you want to move to task_center's so others can also use it.
* Avoid reordering events due to split partition queues Great care is required when splitting queues to avoid losing ordering. We rely on ordering for dedupe so reordering -> dropped kafka messages. * Commit consumer state on rebalance * Review comments * Use spawn_unmanaged * Install prometheus recorder earlier so kafka metrics work
* Avoid reordering events due to split partition queues Great care is required when splitting queues to avoid losing ordering. We rely on ordering for dedupe so reordering -> dropped kafka messages. * Commit consumer state on rebalance * Review comments * Use spawn_unmanaged * Install prometheus recorder earlier so kafka metrics work
* Avoid reordering events due to split partition queues (#2437) * Avoid reordering events due to split partition queues Great care is required when splitting queues to avoid losing ordering. We rely on ordering for dedupe so reordering -> dropped kafka messages. * Commit consumer state on rebalance * Review comments * Use spawn_unmanaged * Install prometheus recorder earlier so kafka metrics work * Rebase changes * Handle panic with empty partitions --------- Co-authored-by: Jack Kleeman <[email protected]>
* Avoid reordering events due to split partition queues Great care is required when splitting queues to avoid losing ordering. We rely on ordering for dedupe so reordering -> dropped kafka messages. * Commit consumer state on rebalance * Review comments * Use spawn_unmanaged * Install prometheus recorder earlier so kafka metrics work * Handle panic with empty partitions
Great care is required when splitting queues to avoid losing ordering. We rely on ordering for dedupe so reordering -> dropped kafka messages.