[DRAFT][18.0] Use session-level advisory lock for requeuing jobs

[guewen]

No description provided.

[OCA-git-bot]

Hi @sbidoul,
some modules you are maintaining are being modified, check this out!

[sbidoul]

I think we had considered that, but worried about potential conflicts with other advisory locks as it can potentially hit a wide range of lock ids. Not sure if we had thought about the two keys locks. As it is mentioned that the one key and two keys lock spaces do not overlap, maybe it's not an issue.

cc/ @AnizR

[sbidoul]

Ah, pgbouncer. Folks using pgbouncer have it with transaction-level pooling which is not compatible with session level advisory locks?

[guewen]

Ah, pgbouncer. Folks using pgbouncer have it with transaction-level pooling which is not compatible with session level advisory locks?

Yes, that's a problem. There is still the option to configure job workers to connect directly to the the database or to use pgdog that now seems to supports session-level advisory locks.

At first sight, that would be acceptable to me with a bump of the major version, but could be interesting to have more insights.

[sbidoul]

As a data point, pgbouncer is the one supported out of the box by cloudnative-pg and we consider using it more to leverage transparent switchover.

[guewen]

As a data point, pgbouncer is the one supported out of the box by cloudnative-pg and we consider using it more to leverage transparent switchover.

Interesting!

When looking for info, I found this saying they consider removing the pooler: cloudnative-pg/cloudnative-pg#6869 (comment). It's a tangeant because it might not change anything, but worth sharing anyway.

[AnizR]

I think we had considered that, but worried about potential conflicts with other advisory locks as it can potentially hit a wide range of lock ids. Not sure if we had thought about the two keys locks. As it is mentioned that the one key and two keys lock

In deed, it was my first approach but, potential conflicts were the main worry and didn't thought of using 2 keys.

I think that we had another issue but can't remind which one.
Don't you have issues when having a rolling back caused by an error within a job?

Otherwise, the PR seems promising!

[guewen]

Don't you have issues when having a rolling back caused by an error within a job?

It shouldn't be an issue because the advisory lock used here is at the session level (connection), so transactional commit/rollbacks have no effect. As for releasing, there is a finally clause around the execution of the job that calls pg_advisory_unlock, so it will be released whether there is an error or not (and automatically released if the connection is closed because of a hard kill).