Skip to content

Full static membership support - #168

Open
whatyouhide wants to merge 3 commits into
elixir-broadway:mainfrom
knocklabs:andrea-static-memberhsip
Open

Full static membership support#168
whatyouhide wants to merge 3 commits into
elixir-broadway:mainfrom
knocklabs:andrea-static-memberhsip

Conversation

@whatyouhide

Copy link
Copy Markdown
Collaborator

No description provided.

Fencing, unlike a regular revoke, doesn't drain the pipeline before the
producer stops: messages already emitted downstream are still in
processors and batchers, and they route through the allocator ETS tables
via :partition_by. Clearing the tables on fence made those lookups raise
ArgumentError and crash the batchers holding in-flight messages.

Since a fenced producer never receives new assignments, the retained
entries are harmless and die with the pipeline.
@slashmili

Copy link
Copy Markdown
Collaborator

Hi @whatyouhide, I was on vacation last week. I'll check the PR this week.

@whatyouhide

Copy link
Copy Markdown
Collaborator Author

@slashmili thank you sounds good! For context, we've forked broadway_kafka since and have been using all of my open PRs (merged to our fork's main) in production for the past ~7 days with no issues.

def resolve_offset(topic, partition, current_offset, offset_reset_policy, config) do
policy = offset_reset_policy_value(offset_reset_policy)

# This is only for testing.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what kind of testing you had in mind? is this still needed for testing?

Comment thread mix.exs
{:brod, "~> 3.16 or ~> 4.0"},
# We need our brod fork until https://github.com/kafka4beam/brod/pull/669
# (fenced static member support) is merged and released upstream.
{:brod, github: "knocklabs/brod", ref: "d16aa8cea37ad9aa8e8591aba0eb2ef814c93e6b"},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I remember correctly, adding this mean we can't publish this package to Hex until this git reference is removed.

I suggest for this PR to be on hold until the PR is merged back to the main stream

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not meant to be merged as is yes, needs to wait for that PR to be merged. Sorry, my bad for not calling that out!

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.

2 participants