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

storcon: make pageserver AZ id mandatory #8856

Merged
merged 5 commits into from
Sep 5, 2024

Conversation

VladLazar
Copy link
Contributor

@VladLazar VladLazar commented Aug 28, 2024

Problem

#8852 introduced a new nullable column for the nodes table: availability_zone_id

Summary of changes

  • Make neon local and the test suite always provide an az id
  • Make the az id field in the ps registration request mandatory
  • Migrate the column to non-nullable and adjust in memory state accordingly
  • Remove the code that was used to populate the az id for pre-existing nodes

This should roll out one deployment after #8852
and we should validate that there are no null entries for the node.availability_zone_id column.

Note

This breaks the test_backward_compatibility test because the current release version of neon local
does not contain #8897. Hence, the migration to make the az id column not null fails.

In production we already have code that detects the az for pageservers and persists them upon registration.
I will merge this PR later into the week and check that all pageservers have persisted an az id.

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.
  • Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
  • If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.

Checklist before merging

  • Do not forget to reformat commit message to not include the above checklist

@VladLazar VladLazar changed the title Vlad/storcon make ps az mandatory storcon: make pageserver AZ id mandatory Aug 28, 2024
Copy link

github-actions bot commented Aug 28, 2024

3822 tests run: 3712 passed, 0 failed, 110 skipped (full report)


Flaky tests (4)

Postgres 16

Postgres 15

Postgres 14

Code coverage* (full report)

  • functions: 32.3% (7430 of 22979 functions)
  • lines: 50.5% (60175 of 119052 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
e10a76f at 2024-09-04T13:25:39.267Z :recycle:

Base automatically changed from vlad/storcon-track-ps-az to main August 28, 2024 17:23
@VladLazar VladLazar force-pushed the vlad/storcon-make-ps-az-mandatory branch 2 times, most recently from b2cffe1 to e4a59e7 Compare September 3, 2024 11:15
@VladLazar VladLazar changed the base branch from main to vlad/neon-local-ps-az September 3, 2024 12:28
@VladLazar VladLazar force-pushed the vlad/storcon-make-ps-az-mandatory branch from e4a59e7 to 959c701 Compare September 3, 2024 12:28
@VladLazar VladLazar force-pushed the vlad/neon-local-ps-az branch from cc593b4 to 3b986b8 Compare September 3, 2024 13:05
@VladLazar VladLazar force-pushed the vlad/storcon-make-ps-az-mandatory branch from 959c701 to 4cd6c65 Compare September 3, 2024 13:06
Base automatically changed from vlad/neon-local-ps-az to main September 3, 2024 14:11
@VladLazar VladLazar force-pushed the vlad/storcon-make-ps-az-mandatory branch 2 times, most recently from 3934ebd to 21d5ccb Compare September 3, 2024 15:45
@VladLazar VladLazar added the backward compatibility breakage Broken backward compatibility. The new version can't handle old data. It'll be hard to deploy label Sep 3, 2024
@VladLazar VladLazar requested review from jcsp and koivunej September 3, 2024 16:56
@VladLazar VladLazar force-pushed the vlad/storcon-make-ps-az-mandatory branch from 21d5ccb to 2f78f1a Compare September 3, 2024 16:56
@VladLazar VladLazar marked this pull request as ready for review September 3, 2024 16:57
@VladLazar VladLazar requested a review from a team as a code owner September 3, 2024 16:57
@VladLazar VladLazar force-pushed the vlad/storcon-make-ps-az-mandatory branch from a669b2c to 2f78f1a Compare September 3, 2024 17:12
@VladLazar VladLazar removed the backward compatibility breakage Broken backward compatibility. The new version can't handle old data. It'll be hard to deploy label Sep 3, 2024
@VladLazar
Copy link
Contributor Author

Need to merge https://github.com/neondatabase/cloud/pull/17191 on the cplane side for e2e tests to pass first.

@VladLazar VladLazar force-pushed the vlad/storcon-make-ps-az-mandatory branch from a7afeef to e10a76f Compare September 4, 2024 12:14
@VladLazar
Copy link
Contributor Author

As of 2024-09-05 all nodes in all regions (apart from the azure region which I couldn't check) have AZs assigned.
I had to manually update a few because deployment was done in reverese order (ps first then storcon).

@VladLazar VladLazar merged commit 04f99a8 into main Sep 5, 2024
67 checks passed
@VladLazar VladLazar deleted the vlad/storcon-make-ps-az-mandatory branch September 5, 2024 18:14
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