Re: snapshot too old, configured by time
От | Kevin Grittner |
---|---|
Тема | Re: snapshot too old, configured by time |
Дата | |
Msg-id | CACjxUsOmT+8D4Z65mrPiOhyfGTV-SMLivMbSs7Jb_f0M6eCosg@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: snapshot too old, configured by time (Alvaro Herrera <alvherre@2ndquadrant.com>) |
Ответы |
Re: snapshot too old, configured by time
(Alvaro Herrera <alvherre@2ndquadrant.com>)
Re: snapshot too old, configured by time (Michael Paquier <michael.paquier@gmail.com>) |
Список | pgsql-hackers |
On Wed, Mar 30, 2016 at 2:22 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > I understand the invasiveness argument, but to me the danger of > introducing new bugs trumps that. The problem is not the current code, > but future patches: it is just too easy to make the mistake of not > checking the snapshot in new additions of BufferGetPage. And you said > that the result of missing a check is silent wrong results from queries > that should instead be cancelled, which seems fairly bad to me. Fair point. > I said that we should change BufferGetPage into having the snapshot > check built-in, except in the cases where a flag is passed; and the flag > would be passed in all cases except those 30-something you identified. > In other words, the behavior in all the current callsites would be > identical to what's there today; we could have a macro do the first > check so that we don't introduce the overhead of a function call in the > 450 cases where it's not needed. In many of the places that BufferGetPage is called there is not a snapshot available. I assume that you would be OK with an Assert that the flag was passed if the snapshot is NULL? I had been picturing what you were requesting as just adding a snapshot parameter and assuming that NULL meant not to check; adding two parameters where the flag explicitly calls that the check is not needed might do more to prevent accidents, but I do wonder how much it would help during copy/paste frenzy. Touching all spots to use the new function signature would be a mechanical job with the compiler catching any errors, so it doesn't seem crazy to refactor that now, but I would like to hear what some others think about this. > Tom said that my proposal would be performance-killing, not that your > patch would be performance-killing. But as I argue above, with my > proposal performance would stay the same, so we're actually okay. > > I don't think nobody disputes that your patch is good in general. > I would be happy with it in 9.6, but I have my reservations about the > aforementioned problem. We have a lot of places in our code where people need to know things that they are not reminded of by the surrounding code, but I'm not about to argue that's a good thing; if the consensus is that this would help prevent future bugs when new BufferGetPage calls are added, I can go with the flow. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
В списке pgsql-hackers по дате отправления: