From 3c92ca2b46a6fd166b4ae556b3d925907c85250c Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 22 Mar 2019 11:47:25 -0600 Subject: [PATCH 1/2] Use (version_id, date) to identify `version_downloads` The `id` column will be removed in the next commit --- mirage/fixtures/version-downloads.js | 3 --- src/bin/update-downloads.rs | 21 ++++++--------------- src/models/download.rs | 3 +-- src/schema.rs | 8 +------- src/views.rs | 1 - 5 files changed, 8 insertions(+), 28 deletions(-) diff --git a/mirage/fixtures/version-downloads.js b/mirage/fixtures/version-downloads.js index 7d3d7fa2409..f7a144d14ae 100644 --- a/mirage/fixtures/version-downloads.js +++ b/mirage/fixtures/version-downloads.js @@ -3,19 +3,16 @@ export default [ { date: '2017-02-10T00:00:00Z', downloads: 2, - id: 3023900, version: 40905, }, { date: '2017-02-10T00:00:00Z', downloads: 1, - id: 3024236, version: 18445, }, { date: '2017-02-11T00:00:00Z', downloads: 1, - id: 3027018, version: 40905, }, ]; diff --git a/src/bin/update-downloads.rs b/src/bin/update-downloads.rs index b31394d5e57..f78f074b014 100644 --- a/src/bin/update-downloads.rs +++ b/src/bin/update-downloads.rs @@ -12,8 +12,6 @@ use cargo_registry::{ use diesel::prelude::*; -static LIMIT: i64 = 1000; - fn main() -> CargoResult<()> { let conn = db::connect_now()?; update(&conn)?; @@ -25,18 +23,11 @@ fn update(conn: &PgConnection) -> QueryResult<()> { use diesel::dsl::now; use diesel::select; - let mut max = Some(0); - while let Some(m) = max { - let rows = version_downloads - .filter(processed.eq(false)) - .filter(id.gt(m)) - .filter(downloads.ne(counted)) - .order(id) - .limit(LIMIT) - .load(conn)?; - collect(conn, &rows)?; - max = rows.last().map(|d| d.id); - } + let rows = version_downloads + .filter(processed.eq(false)) + .filter(downloads.ne(counted)) + .load(conn)?; + collect(conn, &rows)?; // Anything older than 24 hours ago will be frozen and will not be queried // against again. @@ -62,7 +53,7 @@ fn collect(conn: &PgConnection, rows: &[VersionDownload]) -> QueryResult<()> { conn.transaction::<_, diesel::result::Error, _>(|| { // increment the number of counted downloads - update(version_downloads::table.find(download.id)) + update(version_downloads::table.find(download.id())) .set(version_downloads::counted.eq(version_downloads::counted + amt)) .execute(conn)?; diff --git a/src/models/download.rs b/src/models/download.rs index e15db4c62bd..827e32b65a0 100644 --- a/src/models/download.rs +++ b/src/models/download.rs @@ -7,8 +7,8 @@ use crate::views::EncodableVersionDownload; #[derive(Queryable, Identifiable, Associations, Debug, Clone, Copy)] #[belongs_to(Version)] +#[primary_key(version_id, date)] pub struct VersionDownload { - pub id: i32, pub version_id: i32, pub downloads: i32, pub counted: i32, @@ -34,7 +34,6 @@ impl VersionDownload { pub fn encodable(self) -> EncodableVersionDownload { EncodableVersionDownload { - id: self.id, version: self.version_id, downloads: self.downloads, date: self.date.to_string(), diff --git a/src/schema.rs b/src/schema.rs index c89bd7d4eba..0830970c53d 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -815,13 +815,7 @@ table! { /// Representation of the `version_downloads` table. /// /// (Automatically generated by Diesel.) - version_downloads (id) { - /// The `id` column of the `version_downloads` table. - /// - /// Its SQL type is `Int4`. - /// - /// (Automatically generated by Diesel.) - id -> Int4, + version_downloads (version_id, date) { /// The `version_id` column of the `version_downloads` table. /// /// Its SQL type is `Int4`. diff --git a/src/views.rs b/src/views.rs index 00a14a4cddd..9506ac00a05 100644 --- a/src/views.rs +++ b/src/views.rs @@ -66,7 +66,6 @@ pub struct EncodableDependency { #[derive(Serialize, Deserialize, Debug)] pub struct EncodableVersionDownload { - pub id: i32, pub version: i32, pub downloads: i32, pub date: String, From 86ba3335eff50afb21641ee2081b020ad79ff7a6 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 22 Mar 2019 12:36:38 -0600 Subject: [PATCH 2/2] Remove `version_downloads.id` This column is no longer being used, and can be safely removed. Rolling back this migration in production requires creating indexes concurrently, which Diesel CLI doesn't support, so rolling back this migration in production will require queries to be run manually. A blocking version is included for development and testing --- .../down.sql | 28 +++++++++++++++++++ .../up.sql | 6 ++++ 2 files changed, 34 insertions(+) create mode 100644 migrations/2019-03-22-174825_remove_version_downloads_pk/down.sql create mode 100644 migrations/2019-03-22-174825_remove_version_downloads_pk/up.sql diff --git a/migrations/2019-03-22-174825_remove_version_downloads_pk/down.sql b/migrations/2019-03-22-174825_remove_version_downloads_pk/down.sql new file mode 100644 index 00000000000..3e2458b0b86 --- /dev/null +++ b/migrations/2019-03-22-174825_remove_version_downloads_pk/down.sql @@ -0,0 +1,28 @@ +-- Diesel always runs migrations in a transaction, and we can't create indexes +-- concurrently in a transaction. We can't block crate downloads on this, so +-- just raise if we try to run this in production +DO $$ +DECLARE + tmp_index_exists boolean; +BEGIN + tmp_index_exists := (SELECT EXISTS ( + SELECT * FROM pg_index + INNER JOIN pg_class + ON pg_class.oid = pg_index.indexrelid + WHERE relname = 'version_downloads_tmp' + )); + IF NOT tmp_index_exists THEN + IF (SELECT COUNT(*) FROM version_downloads) > 1000 THEN + RAISE EXCEPTION 'Indexes need to be created concurrently in production, manually create them and try again'; + ELSE + ALTER TABLE version_downloads ADD COLUMN id SERIAL; + CREATE UNIQUE INDEX version_downloads_tmp ON version_downloads (id); + CREATE UNIQUE INDEX version_downloads_unique ON version_downloads (version_id, date); + CREATE UNIQUE INDEX index_version_downloads_not_processed ON version_downloads (id) WHERE processed = false; + END IF; + END IF; +END $$; + +ALTER TABLE version_downloads + DROP CONSTRAINT version_downloads_pkey, + ADD CONSTRAINT version_downloads_pkey PRIMARY KEY USING INDEX version_downloads_tmp; diff --git a/migrations/2019-03-22-174825_remove_version_downloads_pk/up.sql b/migrations/2019-03-22-174825_remove_version_downloads_pk/up.sql new file mode 100644 index 00000000000..cecb134929a --- /dev/null +++ b/migrations/2019-03-22-174825_remove_version_downloads_pk/up.sql @@ -0,0 +1,6 @@ +-- ALERT! DESTRUCTIVE MIGRATION +-- DO NOT DEPLOY UNTIL PREVIOUS COMMIT IS DEPLOYED +ALTER TABLE version_downloads + DROP CONSTRAINT version_downloads_pkey, + ADD CONSTRAINT version_downloads_pkey PRIMARY KEY USING INDEX version_downloads_unique, + DROP COLUMN id;