805625 - [crontabber] Add ability to run SET commands before running a function

Status: RESOLVED WONTFIX

(Socorro :: Backend, task)

Description

[Selena Deckelmann :selenamarie :selena]

I am taking all the SET commands related to memory out of our functions for materialized view generation. 

Could we add the ability to run a SET command right before running a function (or group of functions)?

An example command to run would be: 
 SET temp_buffers TO '512MB'

We can only set this command ONCE per session, or we get transaction-ending errors.

Comment 1

[Peter Bengtsson [:peterbe]]

This is tricky because the jobs are quite independent of each other. The dependency chain can work like object oriented programming because descendent jobs won't run unless their parent has. 

If you look at https://github.com/mozilla/socorro/blob/master/socorro/cron/jobs/matviews.py you can see that they "all" depend on `reports-clean` (which is in another file) but we could add a similar thing dedicated to running SET commands. 

If you help me decide the exact dependency web I can write the job app for you.

Comment 2

[Selena Deckelmann :selenamarie :selena]

This system is fine for the purposes of this particular SET command. 

The variable is applied once per SESSION rather than per transaction, so it's ok for it to be set and then a commit to occur (I believe! :D)

The key requirement is that SET temp_buffers is only run ONCE per session. If it is run multiple times, an error occurs and the session needs to be restarted.

What was the thinking behind having the appnames be different than the stored procedure names?

Comment 3

[Peter Bengtsson [:peterbe]]

> The key requirement is that SET temp_buffers is only run ONCE per session.
> If it is run multiple times, an error occurs and the session needs to be
> restarted.
> 
What IS a session? 

> What was the thinking behind having the appnames be different than the
> stored procedure names?

crontabber is built to be generic. A bit biased towards doing things with Postgres but nevertheless quite generic. It just happens to be that most apps are for running postgres stored procedures.

Comment 4

[Selena Deckelmann :selenamarie :selena]

(In reply to Peter Bengtsson [:peterbe] from comment #3)
> > The key requirement is that SET temp_buffers is only run ONCE per session.
> > If it is run multiple times, an error occurs and the session needs to be
> > restarted.
> 
> What IS a session? 

A connection to the database: from the start of a connection to the database to disconnect, and this particular SET command affects all of the BEGIN/COMMIT transactions in between.

> > What was the thinking behind having the appnames be different than the
> > stored procedure names?
> 
> crontabber is built to be generic. A bit biased towards doing things with
> Postgres but nevertheless quite generic. It just happens to be that most
> apps are for running postgres stored procedures.

I can see the usefulness of the *ability* to make names different, but I don't understand the need for making the names different in this case. I think it causes confusion when trying to debug a problem. 

We should make the names the same unless there is a good reason for making them different. I'm not seeing the usefulness of making them different in this case.  What am I not understanding?

Comment 5

[Peter Bengtsson [:peterbe]]

(In reply to Selena Deckelmann :selena from comment #4)
> (In reply to Peter Bengtsson [:peterbe] from comment #3)
> > > The key requirement is that SET temp_buffers is only run ONCE per session.
> > > If it is run multiple times, an error occurs and the session needs to be
> > > restarted.
> > 
> > What IS a session? 
> 
> A connection to the database: from the start of a connection to the database
> to disconnect, and this particular SET command affects all of the
> BEGIN/COMMIT transactions in between.
> 
Then crontabber doesn't have a "native" way to handle this. 
It's not guaranteed which apps will run under one connection. 

The solution must thus be to make the app that runs first of all matviews to also run something like `cursor.execute('SET foo=bar')`

> > > What was the thinking behind having the appnames be different than the
> > > stored procedure names?
> > 
> > crontabber is built to be generic. A bit biased towards doing things with
> > Postgres but nevertheless quite generic. It just happens to be that most
> > apps are for running postgres stored procedures.
> 
> I can see the usefulness of the *ability* to make names different, but I
> don't understand the need for making the names different in this case. I
> think it causes confusion when trying to debug a problem. 
> 
> We should make the names the same unless there is a good reason for making
> them different. I'm not seeing the usefulness of making them different in
> this case.  What am I not understanding?

The app name (e.g. https://github.com/mozilla/socorro/blob/master/socorro/cron/jobs/bugzilla.py#L29) it's important that this doesn't change. Especially if it's a backfill based app. 
You can in fact call it whatever you like. For example: 'my_cool-job-123' as long as it stays like that. If you change the name, it's basically like removing one app and adding another and that can be dangerous in terms of backfilling and remembering when to and not run it again. 

Try to not think of crontabber as a tool for running stored procedures. In the same sense that UNIX cron isn't built to run Postgres stored procedures either. 

Or, are we talking about different things here? Ping me to discuss if you like.

Comment 6

[Selena Deckelmann :selenamarie :selena]

https://bugzilla.mozilla.org/show_bug.cgi?id=796153#c11

Comment 8

[Lonnen :lonnen]

We've moved away from stored procs, and are reducing reliance on crontabber in general. Unlikely to get to this.