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

Upgrade PostgreSQL to 15 #1019

Merged
merged 2 commits into from
Jan 31, 2025
Merged

Upgrade PostgreSQL to 15 #1019

merged 2 commits into from
Jan 31, 2025

Conversation

leplatrem
Copy link
Contributor

No description provided.

@leplatrem leplatrem requested a review from a team as a code owner January 28, 2025 15:42
@leplatrem leplatrem added the dependencies Pull requests that update a dependency file label Jan 28, 2025
@robhudson
Copy link
Collaborator

robhudson commented Jan 29, 2025

I looked at this locally. It fails intermittently.

The 500 resp.content is:

{
  "status": "error",
  "checks": {
    "database": "error"
  },
  "details": {
    "database": {
      "status": "error",
      "level": 40,
      "messages": {
        "db.0002": "Contacts table empty"
      }
    }
  }
}

The health check is counting the number of contacts and getting a -1 which doesn't pass the this assertion, so the test fails.

There's a comment for the count_total_contacts function:

    Since the table is huge, we rely on the PostgreSQL internal
    catalog to retrieve an approximate size efficiently.
    This metadata is refreshed on `VACUUM` or `ANALYSIS` which
    is run regularly by default on our database instances.

Perhaps something changed slightly with those between Postgresql 12 and 15?

This was added for cases where the table has not yet been analyzed and the
estimate is not available... mainly in unit tests.
@robhudson robhudson force-pushed the upgrade-ci-postgresql branch from e26e9f6 to c8f897f Compare January 29, 2025 09:44
@robhudson
Copy link
Collaborator

robhudson commented Jan 29, 2025

Updated with a fallback for cases where the table has not yet been vacuumed or analyzed. In production this fallback will likely never run, but for unit tests it prevents the previous error. Another option is to force analyze the table in the tests but the code here seemed like a safe and straight-forward option.

@robhudson robhudson merged commit 7a647a6 into main Jan 31, 2025
5 checks passed
@robhudson robhudson deleted the upgrade-ci-postgresql branch January 31, 2025 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants