Skip to content

ref(store): Introduce topic column and age-based drain#726

Open
untitaker wants to merge 10 commits into
mainfrom
markusunterwaditzer/stream-1205-implement-contention-mgmt-for-multi-topic-age-based
Open

ref(store): Introduce topic column and age-based drain#726
untitaker wants to merge 10 commits into
mainfrom
markusunterwaditzer/stream-1205-implement-contention-mgmt-for-multi-topic-age-based

Conversation

@untitaker

@untitaker untitaker commented Jun 23, 2026

Copy link
Copy Markdown
Member

This refactor fixes a few remaining bugs when using multiple topics on
postgres:

  1. When two brokers get assignments A:1,B:2 and B:2,A:1 respectively
    (multi-topic, also sharing a DB), the partition filter is useless for
    contention, as each broker filters by partition in (1, 2). The fix is to
    introduce a topic column and make contention management based on (topic, partition), not just partition.

  2. When a topic gets removed from the configuration, its activations are
    orphaned. The fix for this is to extend the partition filter so that rows
    older than a minute are not filtered at all.

  3. When we slice pools (a thing we have planned), we will have many broker
    deployments with their own DB each (slices), but all sharing the same
    consumer group. In this scenario we also want to ensure there are no
    orphaned activations due to partitions moving between slices during
    rebalancing. This is also fixed by the age-based mechanism, see 1.

Because of these bugs, postgres support for multiple topics was completely
disabled in config validation.

See also https://app.notion.com/p/sentry/Contention-and-draining-in-taskbroker-2-0-37e8b10e4b5d803bad50d2eda7d2d1c6

Changes made:

  • Add topic column and wire up indices to match old indices. We used to have an
    index on (partition), we need to replace that with an index on (topic,
    partition). Also update the partition clause to filter on (topic, partition)

  • Refactor assign_partitions: Split it up into assign and revoke, so that the
    consumer logic becomes simpler. It's easier to pass the full
    topic-partition-list from rdkafka directly into the store, and it's more
    correct too (in newer rebalancing modes like KIP-848) than to assume
    partition revocation clears all partitions.

  • Update the partition filter to also contain that mentioned age-based clause.

  • Fix upkeep to actually tag its metrics by topic too, which affects not only
    postgres but is also a bug in sqlite.

Remaining issues:

  • SQLite does not get the topic column, and as a result, all of the upkeep
    metrics are still wrong for it. It's not actually necessary to add the topic
    column because the three bugs mentioned at the beginning don't exist on
    SQLite, and we can live with some busted metrics. It beats having to re-think
    SQLite performance. If it weren't for metrics, the partition column could be
    removed from sqlite too.

  • The old (partition) index still exists in postgres and needs to be removed
    in a followup once we have confirmed the new index works as expected. I did
    some basic tests locally, and it seemed fine.

Some caveats:

  • When there are old activations that match the age-based clause, the upkeep
    metrics could get really weird, since they are tagged by topic and partition.
    Not sure what to do about it.

  • When the topic column is introduced, all existing activations will get
    topic: "" backfilled, so when this change is deployed, all existing
    activations are "orphaned" until they are drained by age. This affects all
    existing postgres pools, of which we have none in production right now.

ref STREAM-1205

@linear-code

linear-code Bot commented Jun 23, 2026

Copy link
Copy Markdown

STREAM-1205

@untitaker untitaker marked this pull request as ready for review June 23, 2026 13:53
@untitaker untitaker requested a review from a team as a code owner June 23, 2026 13:53
Comment thread src/kafka/consumer.rs
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