Skip to content

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
Contributor

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
Contributor

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.

3 participants