Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only consider MIN_ISR requirements if listeners are started #735

Closed
wants to merge 4 commits into from

Conversation

viktorerlingsson
Copy link
Member

@viktorerlingsson viktorerlingsson commented Jul 22, 2024

WHAT is this pull request doing?

Ignores MIN_ISR requirements until listener has been started. Fixes #727

HOW can this pull request be tested?

Specs? Manual steps? Please fill me in.

@viktorerlingsson viktorerlingsson marked this pull request as ready for review July 22, 2024 12:14
@viktorerlingsson
Copy link
Member Author

This seems to work for starting with min_isr > 0. WDYT @carlhoerberg ?

@carlhoerberg
Copy link
Member

I'm just wondering if we should have min_isr at all, Postgres doesnt, Kafka only doesnt ack publishes when min isr isnt fulfilled. Maybe we should do the same? Only not confirm publishes?

But at least this doesnt brakes the "guarantee", if another server with the exact same data dir starts they would end up with the same data anyway.

@viktorerlingsson
Copy link
Member Author

I'm just wondering if we should have min_isr at all, Postgres doesnt, Kafka only doesnt ack publishes when min isr isnt fulfilled. Maybe we should do the same? Only not confirm publishes?

Hmm, yeah, I think that makes sense!

But at least this doesnt brakes the "guarantee", if another server with the exact same data dir starts they would end up with the same data anyway.

👍

@carlhoerberg
Copy link
Member

But probably have to be in combination with storing metadata/definitions in etcd.

@carlhoerberg
Copy link
Member

Fixed in #797

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.

Be able to start a new cluster with min_isr > 0
2 participants