-
Notifications
You must be signed in to change notification settings - Fork 465
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
Remove obsolete sink partition strategy #31463
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test changes lgtm. I like removing old code. I have triggered a nightly run (using https://trigger-ci.dev.materialize.com/): https://buildkite.com/materialize/nightly/builds/11125
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nightly shows some fun panics after upgrading: https://buildkite.com/materialize/nightly/builds/11125
platform-checks-materialized-1 | 2025-02-12T18:26:47.574399Z thread 'main' panicked at src/sql-parser/src/parser.rs:3198:18: internal error: entered unreachable code
Something like this might reproduce it, but haven't run it locally yet: bin/mzcompose --find platform-checks down && bin/mzcompose --find platform-checks run default --scenario=UpgradeEntireMz --check=SinkUpsert
I guess we persist the SQL with the old partition strategy, and then try to parse it in the new version during startup. We should probably still support parsing it so that we can migrate.
aye, definitely looks like that's the case 😁 |
The reproducer I posted above works locally. |
Ditto for me, I believe I've fixed the issue - just waiting for tests to run locally. Thanks @def- ! |
Refactors Kafka sink to only use v1 strategy for publishing. This removes the concept of a partitioning strategy version, which was use for migration from old (v0) to new (v1).
Motivation
Migration for existing users is completed, removing legacy code.
Checklist
$T ⇔ Proto$T
mapping (possibly in a backwards-incompatible way), then it is tagged with aT-proto
label.