Skip to content

Comments

crosscluster/physical: update tokens when altering topology#136122

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
dt:fix-token
Nov 26, 2024
Merged

crosscluster/physical: update tokens when altering topology#136122
craig[bot] merged 1 commit intocockroachdb:masterfrom
dt:fix-token

Conversation

@dt
Copy link
Contributor

@dt dt commented Nov 25, 2024

Release note: none.
Epic: none.

This regressed in #135637 which was assigning all conusmer sub-partitions the whole partition due to sharing the original token.

@dt dt requested a review from msbutler November 25, 2024 19:01
@dt dt requested a review from a team as a code owner November 25, 2024 19:01
@blathers-crl
Copy link

blathers-crl bot commented Nov 25, 2024

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

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

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@msbutler msbutler left a comment

Choose a reason for hiding this comment

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

Are you planning to follow up with a change to remove tokens?

@dt
Copy link
Contributor Author

dt commented Nov 25, 2024

planning to follow up with a change to remove tokens?

It isn't currently on my prioritized backlog.

@dt
Copy link
Contributor Author

dt commented Nov 25, 2024

But it kills me a little that that the DistSQL plan diagram isn't the source of truth as to what the plan will actually do.

Release note: none.
Epic: none.

This regressed in cockroachdb#135637 which was assigning all conusmer sub-partitions the whole partition due
to sharing the original token.
@dt
Copy link
Contributor Author

dt commented Nov 26, 2024

TFTR!

bors r+

@craig craig bot merged commit b6bfe7b into cockroachdb:master Nov 26, 2024
@dt dt deleted the fix-token branch December 9, 2024 16:35
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.

3 participants