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

fix!: Ensure that MeasurementIndexShardId is non-negative #1863

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

SanjayVas
Copy link
Member

BREAKING CHANGE: This is updating an existing Liquibase changeset, and will therefore require manual adjustments wherever it has been rolled out. This should only have rolled out to Halo test environments.

Closes #1862

@SanjayVas SanjayVas requested a review from renjiezh October 16, 2024 17:22
@wfa-reviewable
Copy link

This change is Reviewable

Copy link
Contributor

@renjiezh renjiezh left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @SanjayVas)

@SanjayVas
Copy link
Member Author

SanjayVas commented Oct 16, 2024

Statements to roll back previous schema change:

DROP INDEX MeasurementsByContinuationToken;

ALTER TABLE Measurements
DROP COLUMN MeasurementIndexShardId;

CREATE INDEX MeasurementsByContinuationToken ON Measurements(
  UpdateTime,
  ExternalComputationId,
  State
);

DELETE FROM DATABASECHANGELOG
WHERE
  id = '21'
  AND author = 'sanjayvas'
  AND filename = 'kingdom/spanner/shard-measurements-by-continuation-token.sql';

I've already run this on halo-cmm-head and halo-cmm-qa. It will need to be re-run if the bad schema update is re-applied, e.g. by the Kingdom internal server restarting.

Copy link
Contributor

@renjiezh renjiezh 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! all files reviewed, all discussions resolved (waiting on @stevenwarejones)

Copy link
Collaborator

@stevenwarejones stevenwarejones left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @SanjayVas)

@SanjayVas SanjayVas enabled auto-merge (squash) October 16, 2024 20:34
BREAKING CHANGE: This is updating an existing Liquibase changeset, and will therefore require manual adjustments wherever it has been rolled out. This should only have rolled out to Halo test environments.
@SanjayVas
Copy link
Member Author

I re-ran the statements on halo-cmm-head as it had gotten redeployed without this change.

@SanjayVas SanjayVas merged commit b5c84f8 into main Oct 17, 2024
4 checks passed
@SanjayVas SanjayVas deleted the sanjayvas-shard-id branch October 17, 2024 18:10
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.

Measurements with MeasurementIndexShardId of -1 are not streamed to Herald
4 participants