Skip to content

Comments

Revert "Remove partition strategy migration and remaining bits (#31795)"#31863

Merged
martykulma merged 1 commit intomainfrom
maz-revert-removing-partition-strategy-migration
Mar 12, 2025
Merged

Revert "Remove partition strategy migration and remaining bits (#31795)"#31863
martykulma merged 1 commit intomainfrom
maz-revert-removing-partition-strategy-migration

Conversation

@martykulma
Copy link
Contributor

@martykulma martykulma commented Mar 11, 2025

This reverts commit 2006929 as we need to support upgrade from v0.130.4.

I ran nightly, there were a few failures, but they don't appear to be related to this change. The scalability test failed in SelectStarWorkload.

Motivation

breaks the requirement that Materialize can be upgraded from LTS release

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

This reverts commit 2006929
as we need to support upgrade from v0.130.4.
@martykulma martykulma marked this pull request as ready for review March 11, 2025 23:48
@martykulma martykulma requested a review from a team as a code owner March 11, 2025 23:48
@martykulma martykulma requested a review from aljoscha March 11, 2025 23:48
Copy link
Contributor

@aljoscha aljoscha left a comment

Choose a reason for hiding this comment

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

Does what it says on the tin! 👍

Do you want to retry this later, say figure out why tests failed and then try and merge an updated version of the change?

@martykulma
Copy link
Contributor Author

thanks @aljoscha !!

Do you want to retry this later, say figure out why tests failed and then try and merge an updated version of the change?

It looks like there were 3 failures, but 2 of them require supporting upgrade from 130.

  1. upgrade from LTS (v0.130)
    I introduced a change to remove most of the PARTITION STRATEGY syntax as well as migrate the catalog entries in v0.135. As long as we support upgrading from a version before that, my understanding is that I cannot remove the code that's in this PR.

  2. legacy upgrade tests
    Looking at the logs, these actually upgrade from the previous version (v0.136) AND from LTS version (v0.130). So they run into the same issue issue as 1 above.

  3. testdrive migration tests
    I missed some tests in testdrive-old-kafka-src-syntax that needed a skip-if because the error changed. Easy to fix.

@martykulma martykulma merged commit 78a1332 into main Mar 12, 2025
244 of 248 checks passed
@martykulma martykulma deleted the maz-revert-removing-partition-strategy-migration branch March 12, 2025 11:45
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