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

[cypress] PostgreSQL Connection pool for System Tests #43924

Merged
merged 12 commits into from
Aug 29, 2024

Conversation

muhme
Copy link
Contributor

@muhme muhme commented Aug 15, 2024

Summary of Changes

Use Connection pool for PostgreSQL in System Tests.

Sometimes single specs in the System Tests fail with:

CypressError: `cy.task('queryDB')` failed with the following error:
> duplicate key value violates unique constraint "cpostgresmax_users_pkey"

The error occurs on various specs in the System Tests, especially when running on drone.
It could be reproduced locally in running the System Tests tree times (first with installation, 2nd and 3rd run w/o):

npx cypress run
npx cypress run --spec 'tests/System/integration/administrator/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/site/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/api/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/plugins/**/*.cy.{js,jsx,ts,tsx}'
npx cypress run --spec 'tests/System/integration/administrator/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/site/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/api/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/plugins/**/*.cy.{js,jsx,ts,tsx}'

Checking the session usage in pgAdmin shows the maximum number of 100 (SHOW max_connections) configured sessions appears to have been reached:

before

Therefore the usage of postgres connection pool is implemented with max 10 connections.
The session usage is reduced to max 12 and the error could no more reproduced:

after

Testing Instructions

Having Joomla instance running with PostgreSQL (PDO). Running System Tests and check Sessions tab in pgAdmin.

Actual result BEFORE applying this Pull Request

Heavy session usage during the System Tests. System Tests have sometimes the error duplicate key value violates unique constraint "cpostgresmax_users_pkey".

Expected result AFTER applying this Pull Request

Maximum number of 12 sessions used during the System Tests. System Tests running with all specs passed.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

muhme added 10 commits June 7, 2024 06:47
Consider file permissions when writing configuration in system tests …
merge joomla/joomla-cms branch 4.4-dev
merge head-repository joomla/joomla-cms
merge head repository joomla/joomla-cms
merge from head repository
Sometimes single specs in the System Tests fail with:
    CypressError: `cy.task('queryDB')` failed with the following error:
    > duplicate key value violates unique constraint "cpostgresmax_users_pkey"

The error happens on different specs in System Tests especially running on drone.
It could be reproduced in running the System Tests tree times (first with installation, 2nd and 3rd run w/o):
    npx cypress run
    npx cypress run --spec 'tests/System/integration/administrator/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/site/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/api/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/plugins/**/*.cy.{js,jsx,ts,tsx}'
    npx cypress run --spec 'tests/System/integration/administrator/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/site/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/api/**/*.cy.{js,jsx,ts,tsx},tests/System/integration/plugins/**/*.cy.{js,jsx,ts,tsx}'

Checking the session usage in pgAdmin shows the maximum number of 100 configured sessions appears to have been reached.
Therefore the usage of postgres connection pool is implemented.
The session usage is reduced to max 12 and the error could no more reproduced.
muhme and others added 2 commits August 15, 2024 10:43
- Delete function as only called once
- Renamed to postgresConnectionPool

Contributed by @laoneo
@muhme
Copy link
Contributor Author

muhme commented Aug 20, 2024

I am now happy to have a faster Apple M3 for testing, the 4.4-dev system tests (without installation step) run in only 3:10 with mariadbi. And no wonder that the pgsql error appears here too. But this time with a clear error message:

PostgresError: sorry, too many clients already

With this PR the system tests are passed in 3:07 for pgsql 😄

@muhme
Copy link
Contributor Author

muhme commented Aug 23, 2024

This PR was successfully tested with pgsql in the current branches 4.4-dev, 5.1-dev, 5.2-dev, 5.3-dev and 6.0-dev.

@muhme
Copy link
Contributor Author

muhme commented Aug 23, 2024

Despite the positive tests and good looking charts, the original bug was seen once with 5.2-dev on Apple M3, even though this PR was installed:

Running:  administrator/components/com_privacy/Consent.cy.js                           (28 of 123)


  Test in backend that privacy consent component
    ✓ can view privacy consents (391ms)
    ✓ can invalidate privacy consent (538ms)
    ✓ can invalidate all privacy consents (548ms)
    ✓ can filter by valid consent (545ms)
    ✓ can filter by invalidated consent (552ms)
    ✓ can filter by obsolete consent (537ms)
    ✓ can search username (1109ms)
    ✓ can search user id (1108ms)
    ✓ can search consent id (1040ms)
    ✓ can display correct number of consents (7874ms)
    1) can list by status in ascending order
    2) "after each" hook for "can list by status in ascending order"


  10 passing (16s)
  2 failing

  1) Test in backend that privacy consent component
       can list by status in ascending order:
     CypressError: `cy.task('queryDB')` failed with the following error:

> duplicate key value violates unique constraint "jos52_users_pkey"

In my opinion, this PR is still useful. Without it, it doesn't work at all on an Apple M3 - you get too many clients. But the duplicate key bug needs further investigation (The first interesting detail is that this time it is a different constraint).

But, the problem could not be reproduced in 3 complete runs 5.2-dev and also not with 3 further tests administrator/components/com_privacy/Consent.cy.js. It also did not occur in the other four active branches.

@laoneo laoneo merged commit 53a3390 into joomla:4.4-dev Aug 29, 2024
3 checks passed
@laoneo
Copy link
Member

laoneo commented Aug 29, 2024

Thanks!

@laoneo laoneo added this to the Joomla! 4.4.9! milestone Aug 29, 2024
@muhme
Copy link
Contributor Author

muhme commented Aug 29, 2024

👍 And, definitely it fixes PostgresError: sorry, too many clients already as I see it regulary on M3, it may reduce duplicate key value, definitely do not fix duplicate key value - I am still searching ...

@muhme muhme deleted the fix-postgres-sessions branch August 29, 2024 09:58
heelc29 added a commit to heelc29/joomla-cms that referenced this pull request Sep 1, 2024
laoneo pushed a commit that referenced this pull request Sep 2, 2024
* [cypress] lint mjs files

* fix javascript-cs

caused by #43924
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants