dereuromark / cakephp-queue

Queue plugin for CakePHP - simple, pure PHP and without dependencies.
MIT License
306 stars 137 forks source link

Unable to use plugin with Postgres schema other than 'public' #367

Closed alamote closed 1 year ago

alamote commented 1 year ago

Assuming there is a connection in app.php like

'app' => [
    'className' => Connection::class,
    'driver' => Postgres::class,
    'schema' => 'app',
     ... 
],

and we are using this connection in app_queue.php

'connection' => 'app',

It's expected that plugin will use specified schema to store queued_jobs and queue_processes tables. But when task is processed, plugin tries to update table without schema prefix ('public' schema is used in this case) and expectedly, an error appears that there is no such table (it's located in 'app' schema).

Solution:

Redeclare setTable method in QueuedJobsTable and QueueProcessesTable models like this:

public function setTable(string $table)
{
        $parts = [$table];
        if ($this->getDriverName() === static::DRIVER_POSTGRES) {
            array_unshift($parts, $this->getConnection()->config()['schema'] ?? 'public');
        }
        $this->_table = implode('.', $parts);

        return $this;
}
dereuromark commented 1 year ago

Can you make a PR to show your changes? I dont quite fully get it.

alamote commented 1 year ago

@dereuromark sure, added a common class QueueTable that will set table name to 'app.queued_jobs' (if we are talking about example above) instead of 'queued_jobs'

dereuromark commented 1 year ago

It does feel kind of hacky though I wonder if there isnt a more "framework appropriate" approach to it..

cc @ADmad

alamote commented 1 year ago

@dereuromark But CakePHP's doc for setTable method says:

This can include the database schema name in the form 'schema.table'. If the name must be quoted, enable automatic identifier quoting.

So not sure if it's not "framework appropriate" approach

voronoy commented 1 year ago

This is a very much needed feature.

LordSimal commented 1 year ago

I am unable to reproduce this problem in CakePHP's tests as well as in this plugin's tests using a custom app schema as a DB. Are you certain this is not something you have on your side?

This seems very strang because according to your description

But when task is processed, plugin tries to update table without schema prefix

this would mean, that this ONLY happens when the worker is running and updating the job.

Also: Why woult it be able to fetch a job from queued_jobs but not update it? This is not consistent.

alamote commented 1 year ago

@LordSimal My understanding is that this happens when there are multiple connections in the app. Firstly, Queue Processor is working with connection that is specified in app_queue.php, then some job is using another connection and after that QueuedJobsTable's markJobDone method fails.

I'm getting such excepltion in QueuedJobsTable:687:

SQLSTATE[42P01]: Undefined table: 7 ERROR:  relation "queued_jobs" does not exist
LINE 1: UPDATE queued_jobs SET completed = $1 , progress = $2 WHERE ...
               ^
dereuromark commented 1 year ago

Can you try not to use app_queue config here, but instead use your default config (file) app.php? Note how in next major it will go away with this file (https://github.com/dereuromark/cakephp-queue/blob/cake5/config/app.example.php) in favor of a example file only.

alamote commented 1 year ago

@dereuromark not sure how this can help. If I'm not using app_queue.php (don't configure Queue) then Cannot describe queue_processes. It has 0 columns. error appears (what is expected because there are no queue tables in the default schema). Moving Queue config to app.php (Queue key in return array) leads to the same result when it was in app_queue.php

LordSimal commented 1 year ago

Since we are working with at least 2 different postgres schemas and/or DBs can you try and get a theoretical example working here?

Somehing like:

Connection 1: public

Postgres schema: public Contains tables posts, post_meta etc.

Connection 2: default

Postgres schema: app Contains the queued_jobs and queue_processes tables

The worker run with the configuration associated to Connection 2. But a job being processed by that worker tries to fetch data from Connection 1 via....

LordSimal commented 1 year ago

@alamote what @dereuromark means, is that in the past there was a connection config inside the app_queue.php file as you can see here. https://github.com/dereuromark/cakephp-queue/blob/cake3/config/app_queue.php#L44-L50 But this is not recommended anymore so if you use it, try removing it and see if this solves your problem

alamote commented 1 year ago

@LordSimal

Database structure

Schema app

Schema contains tables:

Schema reminders

Schema contains tables:

Schema cr

Schema contains tables:

Schema public

Schema is not used

App config

Datasources

Use case

Queue Task is working with table in reminders schema (updates sending date) Queue Processor finds job correctly, then Task updates reminder, then Processor tries to update completed and fails

Queue Config

Setting connection to app works only for finding the job, but fails after other connection was used Removing connection key leads to Cannot describe queue_processes. It has 0 columns. error

LordSimal commented 1 year ago

I missunderstood dereuromark there, the connection key is fine there so this shouldn't be the problem. Will have to think about that for a bit what may be the problem here.

LordSimal commented 1 year ago

Do you by chance have any Connection configuration present inside your config directory which specifies no schema?

The error UPDATE queued_jobs SET completed = $1 , progress = $2 WHERE indicates that a table instance used a connection config which doesn't declare a schema.

You could also try and add some debug information just bellow that ->save() call which fails to get some more information on what config it used. Like

public function markJobDone(QueuedJob $job): bool {
    $fields = [
        'progress' => 1,
        'completed' => $this->getDateTime(),
    ];
    $job = $this->patchEntity($job, $fields);

    \Cake\Log\Log::info(print_r($this->_connection->config(), true));
    return (bool)$this->save($job);
}
alamote commented 1 year ago

@LordSimal all connections have a schema name:

\Cake\Datasource\ConnectionManager::getConfig('default')

result = {array[15]} 
 className = "Cake\Database\Connection"
 driver = "Cake\Database\Driver\Postgres"
 name = "default"
 schema = "cr"
...
\Cake\Datasource\ConnectionManager::getConfig('reminders')

result = {array[15]} 
 className = "Cake\Database\Connection"
 driver = "Cake\Database\Driver\Postgres"
 name = "reminders"
 schema = "reminders"
...
\Cake\Datasource\ConnectionManager::getConfig('app')

result = {array[15]} 
 className = "Cake\Database\Connection"
 driver = "Cake\Database\Driver\Postgres"
 name = "app"
 schema = "app"
...

Here is a $this->_connection->config() output before $this->save($job)

result = {array[14]} 
 driver = "Cake\Database\Driver\Postgres"
 name = "app"
 schema = "app"
...
alamote commented 1 year ago

@dereuromark @LordSimal finally found the reason why the issue happens. All problems were due to one parameter in datasource config - missing persistent = false. As I understood, when task is started, a reminders database connection was opened and then QueuedJobs table was trying to execute UPDATE using this existing connection and expectly failed because table is placed in another schema. Solution that I suggested is based on the fact that the schema is always added to the table name, so it does not matter in which connection the query is executed, so this workaround worked for me. Anyway, I added persistent = false to my config and now task is completed and queued job status successfully updated.