[-] reset session before returning connection to the pool

[amihajlovic]

Tasks can leave objects behind after they finish executing.
Most commonly, session scope temporary tables and advisory locks.
Those objects persist after the connection is returned to the pool and can cause issues when the connection is reused.

Simple example:
Start pg_timetable with --cron-workers=1 and create a job SELECT timetable.add_job('tester', '* * * * *', 'create temporary table x();');

First job run will succeed and all other runs during that connections lifetime will fail with: ERROR: relation "x" already exists (SQLSTATE 42P07).

This change also enables using set role in autonomous tasks as the role is reset when connection is returned to the pool.

[pashagolub]

Thanks for your help. That's indeed might be a problem. I would rather start with not so strict reset statements. IMO temp objects, GUC and advisory locks are top priority.

[amihajlovic]

Thanks for your help. That's indeed might be a problem. I would rather start with not so strict reset statements. IMO temp objects, GUC and advisory locks are top priority.

Well, I originaly wanted to resolve set role in autonomous tasks and only after realized you are working on that (among other things).

Regarding priority, temp & cursors share introduce similar issues when leaked. And leaked listens can cause a lot of major issues when pg_notify is used in triggers. Postgres notification queue is fixed size and once filled all those triggers with pg_notify start throwing errors.

[pashagolub]

Indeed, you're right about custom LISTENs... Let me think about it a little :)

[amihajlovic]

On second thought, maybe we should consider a different approach.
There is little benefit in pooling chain executor connections. Timetable is a job agent after all and jobs are not executed multiple times per second.
With that in mind, maybe the safest best approach is to use the pool only for timetable "internal" queries (querying timetable.* objects) and listening to timetable notifications.
For each chain execution we open a new connection. In that case only leakage can occur between two tasks of the same chain and that is perfectly acceptable.
This keeps all chain executions completely isolated from one another, and more importantly isolated from timetable business logic. This will also simplify the code a bit as remote and local execution are handled with the same logic, just different connection strings.

[pashagolub]

Well, we have clients with a lot of jobs, so opening/closing connection will really hurt :(

[amihajlovic]

Separate connection pool for executors? In that case we can safely use DISCARD ALL on release.
That one would be limited by max connections configs.
And we limit timetable internal pool to something like 3 connections as those should be more than enough

[pashagolub]

I believe there is a simpler way. Your first approach sounds good, just need to test that and we're fine. :) Will take a look later

[amihajlovic]

Separate connection pool for executors? In that case we can safely use DISCARD ALL on release. That one would be limited by max connections configs. And we limit timetable internal pool to something like 3 connections as those should be more than enough

And that leads towards dynamic pooling for remote connections. If there are benefits for pooling local chains there is benefit for pooling remote connections as well.

[amihajlovic]

I believe there is a simpler way. Your first approach sounds good, just need to test that and we're fine. :) Will take a look later

👍
It's the easiest one to implement right now.

[pashagolub]

And that leads towards dynamic pooling for remote connections. If there are benefits for pooling local chains there is benefit for pooling remote connections as well.

Might help for a lot of remote tasks, yes. I wonder how many people prefer to use remote chains instead of having a standalone pg_timetable instance for each target database

[coveralls]

Pull Request Test Coverage Report for Build 5519859534

• 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.5%) to 94.939%

---

Totals
Change from base Build 5519918133:	0.5%
Covered Lines:	1632
Relevant Lines:	1719

---

💛 - Coveralls

[pashagolub]

Thanks a lot for your help!