database: add the ability to remove old job records from jobs table

[cmoussa1]

Problem

As it stands right now, the jobs table in the flux-accounting database grows without bound as more and more jobs finish running and are fetched by flux-accounting. However, at a certain point, job records become irrelevant for flux-accounting in terms of calculating job usage and fair-share values.

---

This PR adds a new command to flux-accounting called scrub-old-jobs which takes an integer argument representing the number of weeks to go back. It will remove any job records older than the number of weeks passed
in, e.g flux account scrub-old-jobs 4 will remove any job older than 4 weeks old. If no argument is specified, the default will be 26 weeks (or 6 months).

Some basic sharness tests are added that add some fake job records to the flux-accounting database and run the scrub-old-jobs command to test removing records.

A note about this command and optimizing the memory occupied by the database file by running VACUUM is also added to the flux-accounting guide.

[chu11]

when do you expect users to run this command? I didn't know much about VACUUM but this on the sqlite website concerned me

A VACUUM will fail if there is an open transaction on the database connection that is attempting to run the VACUUM. Unfinalized SQL statements typically hold a read transaction open, so the VACUUM might fail if there are unfinalized SQL statements on the same connection. VACUUM (but not VACUUM INTO) is a write operation and so if another database connection is holding a lock that prevents writes, then the VACUUM will fail.

suggesting to me that scrub should only be done on a non-live system?

[cmoussa1]

I only expect this to be run by admins on an infrequent periodic basis when they no longer require jobs past a certain date. Maybe I could just remove the VACUUM call in this function and make a note in documentation somewhere to point out that admins can manually call VACUUM by connecting to the database themselves and running it if the size of the DB becomes a problem?

[chu11]

manually call VACUUM by connecting to the database themselves and running it if the size of the DB becomes a problem?

Yeah, I think that's a better idea, just documenting how to do it manually. Also, do we know how long VACUUM takes? I suspect it can take a while so maybe putting it in the code is a bad idea??

[cmoussa1]

OK sounds good. VACUUM can take minutes or even tens of minutes depending on how much space the database used to take up (as well as requiring an exclusive lock on the DB), so it probably is a good idea to leave it out of the code completely. 👍 Good point!

I've just force pushed a change to remove the VACUUM call to the function. I also added a note to the flux-accounting guide about passing the --scrub optional argument and also optionally connecting to the database manually and cleaning up occupied space with VACUUM.

[chu11]

Everything looks good o me. But I can't help but wonder if a "flux scrub-job-records" command would be better?

I guess the question is this scrubbing a one-off thing that you expect admins to do (presumably) during a downtime/maintenance/cleanup/something?

Or do you expect people to add this to a cron along with the normal call to flux account-fetch-job-records?

If the former, maybe a new command? If the latter, then I think this is good.

Just a subtle design choice to consider. You would know best on how people would probably use this.

[cmoussa1]

That's a good point. I guess I was expecting this to be added onto the set of cron jobs already running for flux-accounting, i.e every time new jobs were fetched and stored into the flux-accounting DB, old ones could be removed. That way, admins don't have to worry about running a separate command to clean the DB of records themselves.

However, I could absolutely see wanting to make this a one-off command that is only run every so often. In that case, I can just make this a separate command entirely.

@ryanday36 might you have a preference on what the approach is here? Would you rather scrubbing of old jobs occur periodically and in the background (with every call to flux account-fetch-job-records? Or would you like to save something like this for a separate command that you run yourself whenever you want?

[ryanday36]

My first thought is that I don't have a strong preference for flux account-fetch-job-records --scrub versus a separate flux account-scrub-job-records (or similar). I guess if it does turn out to impose some overhead to remove the jobs or VACUUM, it would probably be better have it as a separate command so that we can just run it off hours.

[cmoussa1]

I think that's totally fair. I'll go ahead and modify this PR to instead just introduce a separate command instead of tacking on an optional argument to fetch-job-records. 👍

[chu11]

i think a test for the default (no args == 26 weeks) is missing? Maybe should stick one more test record in there for something > 6 months old.

[chu11]

also, probably should cover the path argument too.

[cmoussa1]

Good point, thanks. I just pushed up a test that does both - specifying a path to the DB as well as the default for scrub-old-jobs (which should remove a simulated job that "finished" over six months ago).

[cmoussa1]

I've restructured this PR to instead add a new command scrub-old-jobs instead of adding an optional argument to flux account-fetch-job-records. The flux-accounting guide has also been updated accordingly to account for the change. Thanks for the feedback so far @chu11 and @ryanday36!

[cmoussa1]

@chu11 is this OK to go in? Sorry, I know you had already approved before I restructured this PR to use a separate command.

[chu11]

oh yes, I think it's good to go!

[cmoussa1]

Thanks! Setting MWP here