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

scpb: ensure IndexZoneConfig fields do not overlap #135933

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

annrpom
Copy link
Contributor

@annrpom annrpom commented Nov 21, 2024

In #134522, we added a fix to ensure that we cleaned up backrefs properly for our IndexZoneConfig element, but in doing so, we created an overlap of field ID 3 between v24.2+. This patch ensures that we do not re-use the same field ID for a different field in our IndexZoneConfig element by marking it as reserved.

Fixes: #133003
Fixes: #135807

Release note: None

@annrpom annrpom requested a review from a team as a code owner November 21, 2024 19:23
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@fqazi fqazi left a comment

Choose a reason for hiding this comment

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

:lgtm:

Nit: For the reserved field add a comment for when we stopped using it

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @annrpom)

@annrpom annrpom added the backport-24.3.x Flags PRs that need to be backported to 24.3 label Nov 21, 2024
@Dedej-Bergin
Copy link
Contributor

-- commits line 12 at r1:
Do we need to add a
Fixes: #134934

Also?

@Dedej-Bergin
Copy link
Contributor

-- commits line 12 at r1:

Previously, Dedej-Bergin (Bergin Dedej) wrote…

Do we need to add a
Fixes: #134934

Also?

Fixes: #134934

In cockroachdb#134522, we added a fix to ensure that we cleaned up backrefs
properly for our `IndexZoneConfig` element, but in doing so, we
created an overlap of field ID `3` between v24.2+.
This patch ensures that we do not re-use the same field ID for
a different field in our IndexZoneConfig element by marking it
as `reserved`.

Fixes: cockroachdb#133003
Fixes: cockroachdb#135807

Release note: None
Copy link
Contributor Author

@annrpom annrpom left a comment

Choose a reason for hiding this comment

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

done

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @Dedej-Bergin and @fqazi)


-- commits line 12 at r1:

Previously, Dedej-Bergin (Bergin Dedej) wrote…

Fixes: #134934

we likely want to leave that one open and ignore it until that branch gets deleted since this does not need to be backported to the rc; apologies for the noise

Copy link
Contributor

@Dedej-Bergin Dedej-Bergin left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @fqazi)

@annrpom
Copy link
Contributor Author

annrpom commented Nov 21, 2024

TFTRs! ('-')7

bors r+

@craig craig bot merged commit 11743ed into cockroachdb:master Nov 21, 2024
22 of 23 checks passed
Copy link

blathers-crl bot commented Nov 21, 2024

Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches.


Issue #135807: branch-release-24.3.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Copy link

blathers-crl bot commented Nov 21, 2024

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error creating merge commit from d0d3a1e to blathers/backport-release-24.3-135933: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 24.3.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-24.3.x Flags PRs that need to be backported to 24.3
Projects
None yet
4 participants