Cylc-flow: Cylc review homogeneous `last active time`

[dwsutherland]

Cylc review user's suite table shows the same last active time for all suites (so ordering is less convenient)

Version: 7.9.1

Expected behavior
last active time should be what it claims to be, and the table ordered by it.

Screenshots

Pull requests welcome!
This is an Open Source project - please consider contributing a bug fix
yourself (please read CONTRIBUTING.md before starting any work though).

[hjoliver]

That's not quite homogeneous - the seconds change from 25 to 26 at the top. Are you sure all the suites weren't just active at once for some reason :grin:

[hjoliver]

Ah, in cylc review it seems (by experiment) "last active time" means the last clock-time at which the suite was running, not the last-updated time of the suite. So I guess we should expect to see all current running suites with the ~same value there?

[dwsutherland]

It use to work.. So I don't know what changed (apart from the Cylc version), or whether it was just a Kupe => Maui oddity

[dwsutherland]

We did go from Rose Bush to Cylc Review. So something in the port between versions maybe

[hjoliver]

What I'm saying is, I'm not sure it's broken. Maybe "last active time" is meant to be "when the workflow was last running" and not "when a task last changed state in the workflow".

[dwsutherland]

It should be the last_updated field provided by the global summary, or the last modification time of the private or public DB (?)

[hjoliver]

Here's the culprit: https://github.com/cylc/cylc-flow/blob/7.8.x/lib/cylc/review.py#L586-L594

(Get's DB last modification time - which on the face of it does seem like the best/easiest way to get this info).

[dwsutherland]

Aha, so maybe it is a Maui thing.. that all the DB files return the same modification time.

[hjoliver]

I did a test run of a handful of suites on my laptop last night and got the same result - even if the suites were held, the "last activity time" was always "now". We might need to check that something isn't constantly and unnecessarily updating the DB...

[dwsutherland]

We might need to check that something isn't constantly and unnecessarily updating the DB...

Yeah, perhaps it's being flagged for update every main loop iteration .. Should be easy to check

[hjoliver]

(Changed label; if this is a bug, it now seems it would be a bug in the scheduler, not cylc-review).

[dwsutherland]

if this is a bug

Oh it's a bug ... That column should (and use to) give you more insight than "is the suite alive", if you had failures or had triggered something, and checked cylc review straight after, that suite would be near the top of the list.

[hjoliver]

Fair point, I tend to agree!

However, I'm wondering if "last modified time" actually gets updated by merely reading an sqlite DB (perhaps because it messes with lock flags inside the DB file), in which case it's not a bug so much as a feature (albeit undesirable) of this implementation :thinking:

Meh, actually that really is a bug. We wouldn't want "last active time" to be updated by merely reading the DB of a stopped suite.

[hjoliver]

I've confirmed (somewhat reassuringly) that merely reading an sqlite DB - at least not via the sqlite3 CLI - does not update st_mtime.

$ stat ~/cylc-run/foo/log/db
Access: 2020-09-17 11:31:53.312030068 +1200
Modify: 2020-09-17 11:31:53.318030190 +1200
Change: 2020-09-17 11:31:53.318030190 +1200

$ sqlite3 ~/cylc-run/foo/log/db 'select * from task_pool;'
1|foo|s|failed|{}|0

$ stat ~/cylc-run/foo/log/db
Access: 2020-09-17 11:32:16.773507348 +1200
Modify: 2020-09-17 11:31:53.318030190 +1200
Change: 2020-09-17 11:31:53.318030190 +1200

So we should check what's happening inside cylc to cause this.

[dwsutherland]

So we should check what's happening inside cylc to cause this.

Could be an inefficiency if so, which will be a good catch.

[hjoliver]

Closed by #3948