Problem/Motivation

Spotted in a daily test run, looks like it might be a random test failure

 Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42P01]: Undefined table: 7 ERROR:  relation "public.test25539123cache_file_parsing" does not exist
       ├ LINE 6: AND pg_attribute.attrelid = '"public"."test25539123cache_fil...  
       ├                                     ^: SELECT pg_attribute.attname AS column_name, format_type(pg_attribute.atttypid, pg_attribute.atttypmod) AS data_type, pg_get_expr(pg_attrdef.adbin, pg_attribute.attrelid) AS column_default
       ├ FROM pg_attribute                                                        
       ├ LEFT JOIN pg_attrdef ON pg_attrdef.adrelid = pg_attribute.attrelid AND pg_attrdef.adnum = pg_attribute.attnum
       ├ WHERE pg_attribute.attnum > 0                                            
       ├ AND NOT pg_attribute.attisdropped                                        
       ├ AND pg_attribute.attrelid = :key::regclass                               
       ├ AND (format_type(pg_attribute.atttypid, pg_attribute.atttypmod) = 'bytea'
       ├ OR pg_get_expr(pg_attrdef.adbin, pg_attribute.attrelid)::text LIKE 'nextval%'); Array
       ├ (                                                                        
       ├     [:key] => "public"."test25539123cache_file_parsing"                  
       ├ )

https://git.drupalcode.org/project/drupal/-/jobs/12009746

Steps to reproduce

Proposed resolution

Catch a more specific exception.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3621147

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Component: cache system » postgresql db driver

OK so the try/catch is already there in ::setMultiple() - which means this might be a failure in the table creation logic itself. Moving to postgresql driver since afaict this only happens on pgsql.

I'm wondering if maybe this is the problem - what if the table doesn't exist, and then table creation fails for a reason other than another process having created the table. Do we need to catch a more specific exception?

    try {
      $database_schema = $this->connection->schema();
      if (!$database_schema->tableExists($this->bin)) {
        $schema_definition = $this->schemaDefinition();
        $database_schema->createTable($this->bin, $schema_definition);
        return TRUE;
      }
    }
    // If another process has already created the cache table, attempting to
    // recreate it will throw an exception. In this case just catch the
    // exception and do nothing.
    catch (DatabaseException) {
      return TRUE;
    }
catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Random test failure

Reran one of the failed jobs I saw and it passed, so this is definitely a random test failure but can only be happening via a real bug somewhere.

catch’s picture

catch
he/him
Primary language English
commented
Component: postgresql db driver » cache system
Status: Active » Needs review

Maybe this is the problem.

catch opened merge request !17019

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
ironnuts’s picture

ironnuts commented

I have read through the code. Wondering if should create test coverage. Have been reading through existing pgsql tests. Not certainof best place to create a test method, maybe here:

https://git.drupalcode.org/project/drupal/-/blob/main/core/modules/pgsql...

If this is a pgsql driver bug not a cache bug then probably test coverage should go inside the pgsql module. Above is a kernel test but a unit test might be better.

longwave ran a gitlab repeat test on another random test failure issue. Could be an alternative way to the above to quality assure the code fix in this issue :
https://git.drupalcode.org/issue/drupal-3620770/-/jobs/11984753

ironnuts’s picture

ironnuts commented

edit: The 1 test failure in the pipeline looks unrelated.

ironnuts’s picture

ironnuts commented

edit: The repeat 100x test could be useful if test coverage is in a kernel test in https://git.drupalcode.org/project/drupal/-/blob/main/core/tests/Drupal/...

catch’s picture

catch
he/him
Primary language English
commented

@oily this isn't a single test that fails randomly, it's a random failure across at least two tests. There may be a real postgresql error behind the failures - we sometimes get database server has gone away for mysql. Either way I very much doubt that the repeat run job will help here because it's likely to be impacted by test concurrency.

ironnuts’s picture

ironnuts commented
Related issues: +#3609986: Improve database exception messages, +#3568040: [PP-1] Convert *Storage() classes' ensureTableExists methods to use SchemaDefinition objects
ironnuts’s picture

ironnuts commented

RTBTC as it is an improvement. The even more specific exceptions in #3609986: Improve database exception messages can be implemented and swapped in when that issue completes.

ironnuts’s picture

ironnuts commented
Status: Needs review » Reviewed & tested by the community
daffie’s picture

daffie commented
Status: Reviewed & tested by the community » Needs review
Related issues: +#3615690: Removal of unnecessary savepoints from PostgreSQL

The PostgreSQL database driver is now mimicking the way MySQL/MariaDB works. It does so by adding a savepoint before each select, insert, update query. After each query the save point is released. Creating a savepoint and releasing a savepoint is a roundtrip to the database. For PostgreSQL we are doing 3 round trips to the database for every query instead of single roundtrip. This is not a great solution. Working with lazy created tables is something that works with MySQL/MariaDB. It does not work with PostgreSQL. PostgreSQL does not like it when you try to do something with a table that does not exist. The best solution for PostgreSQL is to make sure that the table exists BEFORE you start to query it. There is an issue to fix this and it is #3615690: Removal of unnecessary savepoints from PostgreSQL. The issue removes all the added savepoints and changes the lazy creating of database tables. As a side effect does it also improves the performance for Drupal on PostgreSQL a LOT!

ironnuts’s picture

ironnuts commented

Re: #14 changing this to Postponed. However, this issue is critical so by all means if catch or another wants to change to RTBTC, suits me.

ironnuts’s picture

ironnuts commented
Status: Needs review » Postponed
ironnuts’s picture

ironnuts commented

Moved #3615690: Removal of unnecessary savepoints from PostgreSQL from major to critical.