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

Add default_versions database table #8484

Merged
merged 9 commits into from
Apr 25, 2024

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Apr 17, 2024

This PR adds a new database table: default_versions. This table has only two columns: crate_id and version_id.

Once the table has been filled with the update-default-versions admin command it will contain a row for every crate in the database, pointing to the version that will be shown by default by the front-end (not implemented yet) and can be used for other queries where useful (e.g. reverse dependencies).

This calculation has to be performed on the Rust side, since it involves sorting by SemVer and the Postgres extension for SemVer is unfortunately not available on Heroku.

There are four different operations to consider to keep this table in sync:

  • Publishing a new crate: we write the version_id straight into the database since this is the first and therefore default version.
  • Publishing a new version of an existing crate: we update the default version in a background job. this leaves us with a short time of the table pointing to an older, but existing version.
  • Yanking/Unyanking a version: we update the default version in a background job. (see above)
  • Deleting a version (admin action): we update the default version within the same transaction that deletes the versions. the foreign key constraint would otherwise prevent us from deleting a version that is currently a default version.

Note that the code currently does not prevent the update_default_version() fn from running multiple times in parallel for the same crate. I'm open to implementing such a lock, I just haven't found a good way to do it yet. It currently does not deduplicate the background jobs either, but since the job is quite fast it is most likely not needed anyway.

Note also that this table is currently write-only and not used anywhere yet. Once we have had this running in production for a few days we can check whether we see any inconsistencies before we use the data anywhere.

@Turbo87 Turbo87 added C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear A-backend ⚙️ labels Apr 17, 2024
@Turbo87 Turbo87 requested a review from a team April 17, 2024 14:55
@Turbo87 Turbo87 force-pushed the default-versions-table branch 3 times, most recently from 2bb28d0 to 772edb9 Compare April 17, 2024 16:58
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

Attention: Patch coverage is 79.20792% with 42 lines in your changes are missing coverage. Please review.

Project coverage is 88.33%. Comparing base (5f069aa) to head (af423fc).

❗ Current head af423fc differs from pull request most recent head fef072e. Consider uploading reports for the commit fef072e to get more accurate results

Files Patch % Lines
src/admin/delete_version.rs 0.00% 23 Missing ⚠️
src/admin/update_default_versions.rs 0.00% 16 Missing ⚠️
src/admin/yank_version.rs 0.00% 1 Missing ⚠️
src/bin/crates-admin.rs 0.00% 1 Missing ⚠️
src/schema.rs 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8484      +/-   ##
==========================================
- Coverage   88.44%   88.33%   -0.12%     
==========================================
  Files         270      274       +4     
  Lines       26899    27106     +207     
==========================================
+ Hits        23792    23943     +151     
- Misses       3107     3163      +56     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Turbo87 Turbo87 force-pushed the default-versions-table branch from 772edb9 to af423fc Compare April 19, 2024 21:07
@Turbo87 Turbo87 force-pushed the default-versions-table branch from af423fc to fef072e Compare April 25, 2024 12:45
@Turbo87 Turbo87 enabled auto-merge April 25, 2024 12:52
@Turbo87 Turbo87 merged commit eeaae2f into rust-lang:main Apr 25, 2024
6 checks passed
@Turbo87 Turbo87 deleted the default-versions-table branch April 25, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants