On Sat, Feb 24, 2018 at 4:29 AM, Tomas Vondra <tomas.von...@2ndquadrant.com> wrote:
> Hi, > > I see the patch also does throttling by calling vacuum_delay_point(). > Being able to throttle the checksum workers not to affect user activity > definitely seems like a useful feature, no complaints here. > > But perhaps binding it to vacuum_cost_limit/vacuum_cost_delay is not the > best idea? I mean, enabling checksums seems rather unrelated to vacuum, > so it seems a bit strange to configure it by vacuum_* options. > > Also, how am I supposed to set the cost limit? Perhaps I'm missing > something, but the vacuum_delay_point call happens in the bgworker, so > setting the cost limit before running pg_enable_data_checksums() will > get there, right? I really don't want to be setting it in the config > file, because then it will suddenly affect all user VACUUM commands. > > And if this patch gets improved to use multiple parallel workers, we'll > need a global limit (something like we have for autovacuum workers). > > In any case, I suggest mentioning this in the docs. > > Ah yes. I actually have it on my TODO to work on that, but I forgot to put that in the email I sent out. Apologies for that, and thanks for pointing it out! Right now you have to set the limit in the configuration file. That's of course not the way we want to have it long term (but as long as it is that way it should at least be documented). My plan is to either pick it up from the current session that calls pg_enable_data_checksums(), or to simply pass it down as parameters to the function directly. I'm thinking the second one (pass a cost_delay and a cost_limit as optional parameters to the function) is the best one because as you say actually overloading it on the user visible GUCs seems a bit ugly. Once there I think the easiest is to just pass it down to the workers through the shared memory segment. -- Magnus Hagander Me: https://www.hagander.net/ <http://www.hagander.net/> Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
