Re: RFC: Logging plan of the running query - Mailing list pgsql-hackers
From | Julien Rouhaud |
---|---|
Subject | Re: RFC: Logging plan of the running query |
Date | |
Msg-id | ryv7bxrkuycet2rm6yh2igcfdm6cpza3bmu6zlpdx4x5mvgd2r@yjys5ktjz7hv Whole thread Raw |
In response to | Re: RFC: Logging plan of the running query (James Coleman <jtc331@gmail.com>) |
Responses |
Re: RFC: Logging plan of the running query
Re: RFC: Logging plan of the running query |
List | pgsql-hackers |
On Sat, Feb 24, 2024 at 08:56:41AM -0500, James Coleman wrote: > On Fri, Feb 23, 2024 at 10:23 AM Robert Haas <robertmhaas@gmail.com> wrote: > > > > On Fri, Feb 23, 2024 at 7:50 PM Julien Rouhaud <rjuju123@gmail.com> wrote: > > > > > > But it doesn't have to be all or nothing right? I mean each call could say > > > what the situation is like in their context, like > > > CHECK_FOR_INTERRUPTS(GUARANTEE_NO_HEAVYWEIGHT_LOCK | GUARANTEE_WHATEVER), and > > > slowly tag calls as needed, similarly to how we add already CFI based on users > > > report. > > > > Absolutely. My gut feeling is that it's going to be simpler to pick a > > small number of places that are safe and sufficient for this > > particular feature and add an extra call there, similar to how we do > > vacuum_delay_point(). The reason I think that's likely to be better is > > that it will likely require changing only a relatively small number of > > places. If we instead start annotating CFIs, well, we've got hundreds > > of those. That's a lot more to change, and it also inconveniences > > third-party extension authors and people doing back-patching. I'm not > > here to say it can't work; I just think it's likely not the easiest > > path. > > Yes, I suspect it's not the easiest path. I have a small hunch it > might end up paying more dividends in the future: there isn't just one > of these things that is regularly a thorny discussion for the same > reasons each time (basically "no way to trigger this safely from > another backend interrupting another one at an arbitrary point"), and > if we were able to generalize a solution we may have multiple wins (a > very obvious one in my mind is the inability of auto explain to run an > explain at the precise time it's most useful: when statement timeout > fires). Yeah, trying to find a generalized solution seems like worth investing some time to avoid having a bunch of CHECK_FOR_XXX() calls scattered in the code a few years down the road. I might be missing something, but since we already have a ton of macro hacks, why not get another to allow CFI() overloading rather than modifying every single call? Something like that should do the trick (and CFIFlagHandler() is just a naive example with a function call to avoid multiple evaluation, should be replaced with anything that required more than 10s thinking): #define CHECK_FOR_INTERRUPTS_0() \ do { \ if (INTERRUPTS_PENDING_CONDITION()) \ ProcessInterrupts(); \ } while(0) #define CHECK_FOR_INTERRUPTS_1(f) \ do { \ if (INTERRUPTS_PENDING_CONDITION()) \ ProcessInterrupts(); \ \ CFIFlagHandler(f); \ } while(0) #define CHECK_FOR_INTERRUPTS_X(x, f, CFI_IMPL, ...) CFI_IMPL #define CHECK_FOR_INTERRUPTS(...) \ CHECK_FOR_INTERRUPTS_X(, ##__VA_ARGS__, \ CHECK_FOR_INTERRUPTS_1(__VA_ARGS__), \ CHECK_FOR_INTERRUPTS_0(__VA_ARGS__) \ ) We would have to duplicate the current CFI body, but it should never really change, and we shouldn't end up with more than 2 overloads anyway so I don't see it being much of a problem.
pgsql-hackers by date: