Port to PostgreSQL #1085

aldavidson · 2023-06-09T10:57:03Z

Port content-store to run on RDS PostgreSQL, rather than MongoDB.

As of Monday 18th December 2023, all content-store and draft-content-store applications in all environments are running this branch (not main), via the content-store container.
As of Tuesday 2nd Jan at 13:45, the commit history in this branch is now cleaned up, rebased, and ready to be merged into main (force-pushed PR #1199 onto this branch).

This application is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

The Content Store database is being migrated from MongoDB to PostgreSQL. See alphagov/content-store#1085. Nightly backups of the postgres database are now available in S3. By copying them to Google Cloud Platform, we make it possible to adapt the GOV.UK Knowledge Graph to use them.

The content store is being migrated from MongoDB to Postgres. See alphagov/content-store#1085. This is a first attempt to adapt to using the Postgres version. 1. Restore the backup of the Postgres database. 2. Export the `content_items` table as lines of JSON. 3. Import the JSON into MongoDB. 4. Query as before. - Easy to develop, similar to existing steps in the data pipeline - Avoids translating the MongoDB queries into Postgres ones - Not in the spirit of GOV.UK's policy to stop using MongoDB - Extends the data pipeline in both time and complexity - Misses the opportunity to improve the whole pipeline, such as by using the Publishing API database for everything, instead of using the Content Store for some things.

This will allow us to cross-reference PostgreSQL records to MongoDB records post-migration if needed

Support import of doubly-nested mongo date fields Add field mappings for ScheduledPublishingLogEntry and PublishIntent Add mongo_id field to user & scheduled_publishing_log_entry Add `rails_timestamp` method to remove conflicts with ActiveRecord behaviour when doing .insert with some-but-not-all values given Add support for batch_size in JsonImporter

This will allow us to perform side-by-side performance comparisons of the Mongo and Postgres content-stores on the same hardware (e.g. local dev laptop) and prove that the PostgreSQL content-store is at least as performant as the Mongo version.

This improves response times to around 30% of previous values

These will no longer be needed after migration to PostgreSQL.

Some records in the MongoDB have nil values in `created_at` or `updated_at`. ActiveRecord's `timestamps` migration method by default creates these fields without allowing nil values, so we must explicitly add support for this after the fact.

Some records in the old MongoDb have `description` as a simple value, some have it as a Hash. We need to support both, and make sure that we only wrap the given value in a Hash if it's not already like that.

This fixes a bug where unpublished redirects in short URL manager aren't removed from the content store (so continue to work on the website). Users of the content-store API (i.e. publishing-api) might make API calls with values they want to reset provided as `nil`. For example, if you wanted to clear some redirects on a content item, you might do something like: ``` PUT /content/some-made-up-url { ... "redirects": nil ... } ``` The intent of the user of the API is clear here - they want no redirects. However, ContentItem has a default value for redirects: ``` field :redirects, type: Array, default: [] ``` And the rest of the content-store expects this value to be an Array, not to be nil. By passing in potentially nil values in assign_attributes we allow a situation where fields that content-store expects not to be nil (because they have defaults), can be nil. This tends to result in NoMethodErrors, such as this one: ``` NoMethodError undefined method `map' for nil:NilClass redirects = item.redirects.map(&:to_h).map(&:deep_symbolize_keys) ^^^^ /app/app/models/route_set.rb:30:in `from_content_item' /app/app/models/content_item.rb:215:in `route_set' /app/app/models/content_item.rb:225:in `should_register_routes?' /app/app/models/content_item.rb:193:in `register_routes' /app/app/models/content_item.rb:33:in `create_or_replace' /app/app/controllers/content_items_controller.rb:32:in `block in update' ``` I can't think of any valid reason for overriding default attributes with nils, so it feels like calling .compact is the right thing to do here.

The `.any?` method, when called on a Relation, seems to instantiate the objects in the resultset, which is very slow on Kubernetes. It worked fine in Mongoid, but not in ActiveRecord. If we replace this with `.count.positive?`, it's much faster.

Whitehall doesn't do much validation of a given scheduled publishing time. As a result, it can sometimes send us really extreme values for `scheduled_publishing_delay_seconds` (e.g. 400 years into the future). This can cause problems in the importer when Mongo has accepted the value, but PostgreSQL can't. Changing the field type to `bigint` fixes the issue.

...it will fail due to the read-only filesystem in prod. Also, some whitespace-only corrections to the migration

It turns out that you can't call `LogStasher.add_custom_fields` twice. Because Content Store's custom fields config was being run after the govuk_app_config gem's own custom fields config, the gem's config was being overwritten. So, fields like `govuk_request_id` and `varnish_id` weren't appearing in Content Store's controller request logs (but `govuk_dependency_resolution_source_content_id` was). The gem (version 9.7.0) now provides a mechanism for setting custom fields that doesn't overwrite the gem's own settings https://docs.publishing.service.gov.uk/repos/govuk_app_config.html#logger-configuration: ``` GovukJsonLogging.configure do add_custom_fields do |fields| fields[:govuk_custom_field] = request.headers["GOVUK-Custom-Header"] end end ```

An earlier commit on main (#d5422b46 in PR #1136) fixed a subtle issue when overriding default values with nil, by explicitly setting `.created_at` and other attributes from the existing item when it was being replaced. This caused issues on this PostgreSQL branch after rebasing, as ActiveRecord behaves more as expected with respect to `created_at` and therefore the line creating a local `created_at` variable had been removed. This commit reintroduces that variable, and tests now pass again.

aldavidson · 2024-01-02T13:49:05Z

For the record, I have just force-pushed the test branch (from PR #1199) to this branch, as it has a) been rebased onto main, and b) a much cleaner commit history, thanks to @brucebolt for reviewing and tidying that up.

Just in case something were to go wrong with the merge into main, I have tagged the previous head commit of this branch as port-to-postgresql-final-unrebased-version-before-merge-into-main and pushed it to remote, as that's the commit which is currently running in production and all other environments. So in a worst-case scenario of needing to rollback, we can still retrieve that.

aldavidson added the do not merge label Jun 9, 2023

aldavidson mentioned this pull request Jun 9, 2023

Add ECR repo for content-store-postgresql-branch alphagov/govuk-infrastructure#889

Merged

aldavidson marked this pull request as ready for review June 9, 2023 11:23

aldavidson force-pushed the port-to-postgresql branch from 177d522 to e258cd3 Compare June 9, 2023 11:29

aldavidson force-pushed the port-to-postgresql branch 2 times, most recently from 38aab29 to d390f76 Compare June 29, 2023 11:52

aldavidson force-pushed the port-to-postgresql branch 2 times, most recently from e7bbce4 to 88bf1aa Compare July 10, 2023 14:52

aldavidson mentioned this pull request Aug 30, 2023

Bring in latest changes from main #1144

Merged

nacnudus mentioned this pull request Oct 30, 2023

Include the new postgres content store alphagov/govuk-s3-mirror#47

Merged

nacnudus mentioned this pull request Oct 31, 2023

Derive MongoDB content store from Postgres alphagov/govuk-knowledge-graph-gcp#546

Merged

aldavidson and others added 20 commits December 27, 2023 14:26

Add DatabaseCleaner config to the Pact tests

ea2f82b

Add _id field to content_items, to store imported mongo ID

29aa93a

This will allow us to cross-reference PostgreSQL records to MongoDB records post-migration if needed

Add performance:compare_hosts rake task

06208fb

This will allow us to perform side-by-side performance comparisons of the Mongo and Postgres content-stores on the same hardware (e.g. local dev laptop) and prove that the PostgreSQL content-store is at least as performant as the Mongo version.

Add optimised indexes on JSONB array routes/redirects columns

797853a

This improves response times to around 30% of previous values

Remove mongo export files

6eb07aa

These will no longer be needed after migration to PostgreSQL.

Allow JsonImporter to import all files in a given dir, and add specs

9c34535

Fix issue that can cause doubly-nested description Hashes on PostgreSQL

a4e4e96

Some records in the old MongoDb have `description` as a simple value, some have it as a Hash. We need to support both, and make sure that we only wrap the given value in a Hash if it's not already like that.

Make JsonImporter only import gzipped files

fbaa77d

Don't dump the schema after running migrations in production

f2f67e3

...it will fail due to the read-only filesystem in prod. Also, some whitespace-only corrections to the migration

Fix error when auth_bypass_ids is nil

624012b

Upgrade rails from 7.0.8 to 7.1.2

29ba644

Retain compatibility with #bbe2fda / PR #1152 on main

bf06dd0

update comment about exposing id outside the model

7919408

aldavidson force-pushed the port-to-postgresql branch from 32b571a to 7919408 Compare January 2, 2024 13:44

aldavidson changed the title ~~[Do not merge] Port to PostgreSQL~~ Port to PostgreSQL Jan 2, 2024

aldavidson requested review from richardTowers and brucebolt January 2, 2024 13:52

aldavidson removed the do not merge label Jan 2, 2024

brucebolt approved these changes Jan 2, 2024

View reviewed changes

aldavidson merged commit e383e17 into main Jan 2, 2024
19 checks passed

aldavidson deleted the port-to-postgresql branch January 2, 2024 14:09

aldavidson mentioned this pull request Jan 2, 2024

Remove README note about deployment in transitional state #1206

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port to PostgreSQL #1085

Port to PostgreSQL #1085

aldavidson commented Jun 9, 2023 •

edited

Loading

aldavidson commented Jan 2, 2024 •

edited

Loading

Port to PostgreSQL #1085

Port to PostgreSQL #1085

Conversation

aldavidson commented Jun 9, 2023 • edited Loading

aldavidson commented Jan 2, 2024 • edited Loading

aldavidson commented Jun 9, 2023 •

edited

Loading

aldavidson commented Jan 2, 2024 •

edited

Loading