cylc.rundb: add task jobs table and refactor

[matthewrmshin]

• Simplify by removing threading logic.
• Group insert and similar update statements, and write with executemany for efficiency.
  • A single queue per table for inserts.
  • A queue for each unique update statement template.
• On data retrieval, use single select statement to get every relevant items.
• Die immediately on any problem with private database to guarantee integrity.
  • Database can only be out of sync by a single iteration on the main loop.
• Unlimited number of retries for public database.
  Retry occurs on next iteration of the main loop - no sleep required.
  Suite can only be brought down if queues become so big that memory runs out.
  This is unlikely, because things should recover sooner or later.
• On restart,
  • Vacuum private database
  • Copy private database into public database to ensure they are in sync.
• Add new task_jobs table.
• Add new broadcasts table to record broadcast settings in searchable form.
  Remove broadcast_settings table, which stored unsearchable pickles.

[matthewrmshin]

I have done some stress tests on this, and I have yet to see a "database is locked" error on writing to the databases.

(This is ready for initial comments, but not for merge, because I want to add some automated tests for the new tables.)

[matthewrmshin]

(Stress test based on https://gist.github.com/matthewrmshin/c5e301e060a1987afaf7 with the succeeded handler of each task doing a query on the public database. A loop is also set up to run continuously to query the public database on a separate terminal.)

[matthewrmshin]

@cylc/core please review.

[arjclark]

why not just do a .join on primary_keys as you do for column_str_list?

[matthewrmshin]

Because not every table has a primary key?

[arjclark]

@matthewrmshin - from a quick pass through, I can't work out how it will cope with a temporary database lock (hence retries in the old system). Also, I don't think this is backwards compatible - you need to create the new table in an existing suite database.

[matthewrmshin]

from a quick pass through, I can't work out how it will cope with a temporary database lock (hence retries in the old system)

For private database, it does not retry because it should never be necessary.

For public database, it retries when the main loop calls self.pool.process_queued_db_ops() again in the next iteration.

[matthewrmshin]

Also, I don't think this is backwards compatible - you need to create the new table in an existing suite database.

No, but do we need to support backward compatibility for restart? I think it is always safer to stick with the same version of cylc on a restart. (Rose's suite commands reinforce that any way.)

[dpmatthews]

For public database, it retries when the main loop calls self.pool.process_queued_db_ops() again in the next iteration.

Is there a limit on the number of retries? Other suites may be relying on the public database to check for task completions (and the hook scripts currently rely on it) so we can't afford to let it get too behind..

[hjoliver]

@matthewrmshin - regarding your stress testing, out of interest do you see locks when doing the same testing on current master? Also, your gist link seems to be broken.

[hjoliver]

Is there a limit on the number of retries? Other suites may be relying on the public database to check for task completions (and the hook scripts currently rely on it) so we can't afford to let it get too behind..

This a tough call... couldn't it be argued that important suites should carry on no matter what, unless their private db is somehow locked or broken? We could just warn the suite operator of the problem, and recommend a restart as soon as possible.

[hjoliver]

do we need to support backward compatibility for restart?

So far we have tried to do that.

[arjclark]

do we need to support backward compatibility for restart?

From the way we do things here during a PS, yes we do.

[arjclark]

...Other suites may be relying on the public database to check for task completions (and the hook scripts currently rely on it) so we can't afford to let it get too behind..

This a tough call... couldn't it be argued that important suites should carry on no matter what, unless their private db is somehow locked or broken? We could just warn the suite operator of the problem, and recommend a restart as soon as possible.

Problem is though, if the external db gets too out of sync then it's useless for external triggering, current hook scripts, rose-bush like utilities etc. Re-synchronisation then becomes an increasingly large problem on the restart as well - and by the looks of things there's no dump any more(?) without that the external will remain out of sync on a restart.

[arjclark]

You've also removed user only access to the private database, can you put that back in please? It ensured only the user's suite process and their suite processes so was a very useful feature to have.

[matthewrmshin]

Is there a limit on the number of retries? Other suites may be relying on the public database to check for task completions (and the hook scripts currently rely on it) so we can't afford to let it get too behind..

This a tough call... couldn't it be argued that important suites should carry on no matter what, unless their private db is somehow locked or broken? We could just warn the suite operator of the problem, and recommend a restart as soon as possible.

I think we can implement a logic where, after say 100 updates with incomplete operations, we simply copy the content of the private database into a temporary location next to the public database, and then rename that to become the new public database. Whatever connection locking the old public database will still be connected to its inode, but it should no longer affect future accesses. (Hopefully, that process will soon recover to give up the lock, and then free up the disk space.)

regarding your stress testing, out of interest do you see locks when doing the same testing on current master? Also, your gist link seems to be broken.

I can't get write to fail with a lock at all in either case. However, I think external read has become more robust with this branch.

Gist link now fixed.

the external will remain out of sync on a restart.

No, that won't be case any more, because we'll be copying the private into the public on restart.

[matthewrmshin]

Implemented recovery of public database file from private database file after 100 failed attempts.

• Reduced automatic connection time out to 0.2s.
• Auto recovery tested with new battery test.

All other issues should be addressed.

• Create tables if they don't exist.
• chmod 0600 for private database.

[matthewrmshin]

Branch rebased and squashed.

[hjoliver]

I like the lock-breaking recovery mechanism, tested with @dpmatthews' read_lock.c.

[hjoliver]

Tested with a suite of task pool size ~4500, task_pool.process_queued_db_ops() typically took about 1-1.5 seconds, no problems found.

Simplify by removing threading logic.

I take it you think there's no advantage in using a thread for db ops - even in very big, busy suites? (well it's clearly not a problem in my aforementioned test suite).

Vacuuming a db copied from one of our long-running suites (~300 MB) took almost 30 sec; might be worth printing info to stdout when doing this?

@arjclark - are you happy with the changes on this branch?

[matthewrmshin]

I take it you think there's no advantage in using a thread for db ops - even in very big, busy suites? (well it's clearly not a problem in my aforementioned test suite).

For the private database, there is no real advantage, because we want it to be like the state dump - always in sync with what is current. For the public database, which does not need to be fully in sync, there is some advantage of using a separate thread/process, if writing to it becomes a bottleneck. At the moment, I don't see any advantage as it does not appear to be a bottleneck.

(If writing to disk becomes a bottleneck for a busy suite, I would imagine that the priority should be to move the logic for writing the job files to separate threads/processes (or the process pool) - writes to multiple files in separate directories, before worrying about the public suite database - writes to a single file.)

Vacuuming a db copied from one of our long-running suites (~300 MB) took almost 30 sec; might be worth printing info to stdout when doing this?

I'll add some print statements.

[hjoliver]

For the private database, there is no real advantage, ...

This sort of thing should be documented in the code IMO - in case it's not obvious to future developers (or future us). Can you add a couple of comments?

[matthewrmshin]

Can you add a couple of comments?

Done.

Branch rebased.

[hjoliver]

My test battery passes.

[arjclark]

@matthewrmshin - noting on ticket as discussed:

• test battery passing in my environment
• haven't been able to break via manual hacking around at this point
• my only disagreements with what's now implemented are stylistic rather than functional

As discussed, to be on the safe side, I'd like this rolled out to our early adopters/general users for a while before we go for operational implementation to make sure we haven't missed anything (I can't see that we have but there's usually something)