Re: [HACKERS] Snapshot too old logging

2016-11-18 Thread Robert Haas
On Fri, Nov 18, 2016 at 11:49 AM, Brad DeJong wrote: > On Wed, Nov 16, 2016 at 6:43 AM, Robert Haas wrote: >> I think I was suggesting: One or more rows required by this query may >> already have been removed from "%s". > > I keep reading that as "you have data corruption

Re: [HACKERS] Snapshot too old logging

2016-11-18 Thread Brad DeJong
On Wed, Nov 16, 2016 at 6:43 AM, Robert Haas wrote: > I think I was suggesting: One or more rows required by this query may > already have been removed from "%s". I keep reading that as "you have data corruption because something removed rows that your query needs" rather than "this query took

Re: [HACKERS] Snapshot too old logging

2016-11-16 Thread Kevin Grittner
On Wed, Nov 16, 2016 at 6:12 AM, Magnus Hagander wrote: > On Tue, Nov 15, 2016 at 9:23 PM, Alvaro Herrera > wrote: >> Can this happen for relation types other than tables, say materialized >> views? (Your suggested wording omits relation type so

Re: [HACKERS] Snapshot too old logging

2016-11-16 Thread Robert Haas
On Tue, Nov 15, 2016 at 2:25 PM, Magnus Hagander wrote: > On Tue, Nov 15, 2016 at 8:22 PM, Robert Haas wrote: >> >> On Tue, Nov 15, 2016 at 2:21 PM, Kevin Grittner wrote: >> > On Tue, Nov 15, 2016 at 12:43 PM, Robert Haas

Re: [HACKERS] Snapshot too old logging

2016-11-16 Thread Magnus Hagander
On Tue, Nov 15, 2016 at 9:23 PM, Alvaro Herrera wrote: > Magnus Hagander wrote: > > On Tue, Nov 15, 2016 at 8:22 PM, Robert Haas > wrote: > > > > > On Tue, Nov 15, 2016 at 2:21 PM, Kevin Grittner > wrote: > > > > > That

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Brad DeJong
Magnus wrote: > Just to be clear, you're suggesting 'One or more rows may have already been > removed from "%s"? Perhaps just 'This query attempted to access a page in "%s" that was modified after the snapshot was acquired.'

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Alvaro Herrera
Magnus Hagander wrote: > On Tue, Nov 15, 2016 at 8:22 PM, Robert Haas wrote: > > > On Tue, Nov 15, 2016 at 2:21 PM, Kevin Grittner wrote: > > > That particular language would be misleading. All we know about > > > the page is that it was modified

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Magnus Hagander
On Tue, Nov 15, 2016 at 8:22 PM, Robert Haas wrote: > On Tue, Nov 15, 2016 at 2:21 PM, Kevin Grittner wrote: > > On Tue, Nov 15, 2016 at 12:43 PM, Robert Haas > wrote: > > > >> I think it would be better not to include either the

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Robert Haas
On Tue, Nov 15, 2016 at 2:21 PM, Kevin Grittner wrote: > On Tue, Nov 15, 2016 at 12:43 PM, Robert Haas wrote: > >> I think it would be better not to include either the snapshot or the >> block number, and just find some way to reword the error message so

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Kevin Grittner
On Tue, Nov 15, 2016 at 12:43 PM, Robert Haas wrote: > I think it would be better not to include either the snapshot or the > block number, and just find some way to reword the error message so > that it mentions which relation was involved without implying that all >

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Magnus Hagander
On Tue, Nov 15, 2016 at 7:43 PM, Robert Haas wrote: > On Tue, Nov 15, 2016 at 1:30 PM, Magnus Hagander > wrote: > > On Tue, Nov 15, 2016 at 7:27 PM, Robert Haas > wrote: > >> > >> On Tue, Nov 15, 2016 at 1:18 PM, Magnus

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Kevin Grittner
On Tue, Nov 15, 2016 at 12:45 PM, Tom Lane wrote: > Magnus Hagander writes: >> Is there value in showing which snapshot as well? Something like: >> DETAIL: snapshot is too old to access relation > > Snapshots don't have names, and I can't think of a

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Tom Lane
Magnus Hagander writes: > Is there value in showing which snapshot as well? Something like: > DETAIL: snapshot is too old to access relation Snapshots don't have names, and I can't think of a useful way of identifying them to users. regards, tom

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Robert Haas
On Tue, Nov 15, 2016 at 1:30 PM, Magnus Hagander wrote: > On Tue, Nov 15, 2016 at 7:27 PM, Robert Haas wrote: >> >> On Tue, Nov 15, 2016 at 1:18 PM, Magnus Hagander >> wrote: >> > Is there value in showing which snapshot as well?

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Brad DeJong
On Tue, Nov 15, 2016 at 12:20 PM Magnus Hagander wrote: > Is there value in showing the snapshot as well? I don't think so. Knowing the relname let's you look at your report/job and figure out if the access to that relation can be moved. Having the exact snapshot version isn't going to change

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Magnus Hagander
On Tue, Nov 15, 2016 at 7:27 PM, Robert Haas wrote: > On Tue, Nov 15, 2016 at 1:18 PM, Magnus Hagander > wrote: > > Is there value in showing which snapshot as well? Something like: > > DETAIL: snapshot is too old to access relation > > IIUC, the

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Robert Haas
On Tue, Nov 15, 2016 at 1:18 PM, Magnus Hagander wrote: > Is there value in showing which snapshot as well? Something like: > DETAIL: snapshot is too old to access relation IIUC, the granularity is per-block, not per-relation, so that might be misleading. -- Robert Haas

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Brad DeJong
On Tue, Nov 15, 2016 at 10:30 AM, Tom Lane wrote: > > Kevin Grittner writes: > > On Tue, Nov 15, 2016 at 3:38 AM, Magnus Hagander > wrote: > >> Is there a reason why we don't log which relation triggered the > >> snapshot too old error when it happens? >

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Magnus Hagander
On Tue, Nov 15, 2016 at 5:29 PM, Tom Lane wrote: > Kevin Grittner writes: > > On Tue, Nov 15, 2016 at 3:38 AM, Magnus Hagander > wrote: > >> Is there a reason why we don't log which relation triggered the > snapshot too > >> old error

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Tom Lane
Kevin Grittner writes: > On Tue, Nov 15, 2016 at 3:38 AM, Magnus Hagander wrote: >> Is there a reason why we don't log which relation triggered the snapshot too >> old error when it happens? > I would probably not want to mess with the text of the error >

Re: [HACKERS] Snapshot too old logging

2016-11-15 Thread Kevin Grittner
On Tue, Nov 15, 2016 at 3:38 AM, Magnus Hagander wrote: > Is there a reason why we don't log which relation triggered the snapshot too > old error when it happens? > > Since we do have Relation available in TestForOldSnapshot_impl(), shouldn't > we be able to include it? > >

[HACKERS] Snapshot too old logging

2016-11-15 Thread Magnus Hagander
Is there a reason why we don't log which relation triggered the snapshot too old error when it happens? Since we do have Relation available in TestForOldSnapshot_impl(), shouldn't we be able to include it? Having that in the log message would be very helpful, but is there a reason why it

Re: [HACKERS] snapshot too old, configured by time

2016-05-02 Thread Bruce Momjian
On Mon, May 2, 2016 at 03:50:36PM -0500, Kevin Grittner wrote: > >> Also, it seems we have similar behavior already in applying WAL on the > >> standby --- we delay WAL replay when there is a long-running > >> transaction. Once the time expires, we apply the WAL. Do we cancel the > >>

Re: [HACKERS] snapshot too old, configured by time

2016-05-02 Thread Kevin Grittner
On Sun, May 1, 2016 at 11:54 PM, Bruce Momjian wrote: > On Sat, Apr 23, 2016 at 10:20:19AM -0400, Bruce Momjian wrote: >> On Sat, Apr 23, 2016 at 12:48:08PM +0530, Amit Kapila wrote: >>> On Sat, Apr 23, 2016 at 8:34 AM, Bruce Momjian wrote: I kind of

Re: [HACKERS] snapshot too old, configured by time

2016-05-01 Thread Bruce Momjian
On Sat, Apr 23, 2016 at 10:20:19AM -0400, Bruce Momjian wrote: > On Sat, Apr 23, 2016 at 12:48:08PM +0530, Amit Kapila wrote: > > On Sat, Apr 23, 2016 at 8:34 AM, Bruce Momjian wrote: > > > > > > I kind of agreed with Tom about just aborting transactions that held > > >

Re: [HACKERS] snapshot too old, configured by time

2016-04-25 Thread Alexander Korotkov
On Sat, Apr 23, 2016 at 5:20 PM, Bruce Momjian wrote: > On Sat, Apr 23, 2016 at 12:48:08PM +0530, Amit Kapila wrote: > > On Sat, Apr 23, 2016 at 8:34 AM, Bruce Momjian wrote: > > > > > > I kind of agreed with Tom about just aborting transactions that held > >

Re: [HACKERS] snapshot too old, configured by time

2016-04-23 Thread Bruce Momjian
On Sat, Apr 23, 2016 at 12:48:08PM +0530, Amit Kapila wrote: > On Sat, Apr 23, 2016 at 8:34 AM, Bruce Momjian wrote: > > > > I kind of agreed with Tom about just aborting transactions that held > > snapshots for too long, and liked the idea this could be set per > > session, but

Re: [HACKERS] snapshot too old, configured by time

2016-04-23 Thread Amit Kapila
On Sat, Apr 23, 2016 at 8:34 AM, Bruce Momjian wrote: > > On Tue, Apr 19, 2016 at 07:38:04AM -0400, Robert Haas wrote: > > 2. Without this feature, you can kill sessions or transactions to > > control bloat, but this feature is properly thought of as a way to > > avoid bloat

Re: [HACKERS] snapshot too old, configured by time

2016-04-22 Thread Bruce Momjian
On Tue, Apr 19, 2016 at 07:38:04AM -0400, Robert Haas wrote: > 2. Without this feature, you can kill sessions or transactions to > control bloat, but this feature is properly thought of as a way to > avoid bloat *without* killing sessions or transactions. You can let > the session live, without

Re: [HACKERS] snapshot too old, configured by time

2016-04-21 Thread Kevin Grittner
On Wed, Apr 20, 2016 at 8:50 AM, Kevin Grittner wrote: > In case anyone notices some code left at the bottom of bufmgr.h > related to inline functions, that was left on purpose, because I am > pretty sure that the fix for the performance regression observed > when the

Re: [HACKERS] snapshot too old, configured by time

2016-04-20 Thread Kevin Grittner
On Tue, Apr 19, 2016 at 12:49 PM, Alvaro Herrera wrote: > Well, it seems I'm outvoted. The no-op change to attempt to force an explicit choice of whether to test for snapshot age after calling BufferGetPage() has been reverted. This eliminates about 500 back-patching

Re: [HACKERS] snapshot too old, configured by time

2016-04-19 Thread Alvaro Herrera
Kevin Grittner wrote: > On Tue, Apr 19, 2016 at 11:02 AM, Alvaro Herrera > wrote: > > Robert Haas wrote: > > > >> That wouldn't have fixed my problem, which involved rebasing a patch. > > > > True. I note that it's possible to munge a patch mechanically to sort > > out

Re: [HACKERS] snapshot too old, configured by time

2016-04-19 Thread Kevin Grittner
On Tue, Apr 19, 2016 at 11:02 AM, Alvaro Herrera wrote: > Andres Freund wrote: >> On 2016-04-19 12:03:22 -0300, Alvaro Herrera wrote: > Since this change to BufferGetPage() has caused severe back-patch pain for at least two committers so far, I will revert that

Re: [HACKERS] snapshot too old, configured by time

2016-04-19 Thread Alvaro Herrera
Andres Freund wrote: > On 2016-04-19 12:03:22 -0300, Alvaro Herrera wrote: > > > Since this change to BufferGetPage() has caused severe back-patch > > > pain for at least two committers so far, I will revert that (very > > > recent) change to this patch later today unless I hear an > > >

Re: [HACKERS] snapshot too old, configured by time

2016-04-19 Thread Robert Haas
On Tue, Apr 19, 2016 at 11:03 AM, Alvaro Herrera wrote: > Kevin Grittner wrote: >> On Tue, Apr 19, 2016 at 6:38 AM, Robert Haas wrote: >> >> > The right thing to do about that is just change it back to the >> > way Kevin had it originally. >> >>

Re: [HACKERS] snapshot too old, configured by time

2016-04-19 Thread Andres Freund
On 2016-04-19 12:03:22 -0300, Alvaro Herrera wrote: > Kevin Grittner wrote: > > On Tue, Apr 19, 2016 at 6:38 AM, Robert Haas wrote: > > > > > The right thing to do about that is just change it back to the > > > way Kevin had it originally. > > > > Since this change to

Re: [HACKERS] snapshot too old, configured by time

2016-04-19 Thread Alvaro Herrera
Kevin Grittner wrote: > On Tue, Apr 19, 2016 at 6:38 AM, Robert Haas wrote: > > > The right thing to do about that is just change it back to the > > way Kevin had it originally. > > Since this change to BufferGetPage() has caused severe back-patch > pain for at least two

Re: [HACKERS] snapshot too old, configured by time

2016-04-19 Thread Kevin Grittner
On Tue, Apr 19, 2016 at 6:38 AM, Robert Haas wrote: > The right thing to do about that is just change it back to the > way Kevin had it originally. Since this change to BufferGetPage() has caused severe back-patch pain for at least two committers so far, I will revert

Re: [HACKERS] snapshot too old, configured by time

2016-04-19 Thread Robert Haas
On Tue, Apr 19, 2016 at 1:23 AM, Michael Paquier wrote: > On Tue, Apr 19, 2016 at 3:14 AM, Tom Lane wrote: >> Or in short: this is a whole lot further than I'm prepared to go to >> satisfy one customer with a badly-designed application. And from

Re: [HACKERS] snapshot too old, configured by time

2016-04-18 Thread Michael Paquier
On Tue, Apr 19, 2016 at 3:14 AM, Tom Lane wrote: > Or in short: this is a whole lot further than I'm prepared to go to > satisfy one customer with a badly-designed application. And from what > I can tell from the Feb 2015 discussion, that's what this has been > written for.

Re: [HACKERS] snapshot too old, configured by time

2016-04-18 Thread Tom Lane
Kevin Grittner writes: > On Mon, Apr 18, 2016 at 8:50 AM, Tom Lane wrote: >> Surely there was another way to get a similar end result without >> mucking with things at the level of BufferGetPage. > To get the feature that some customers have been

Re: [HACKERS] snapshot too old, configured by time

2016-04-18 Thread Kevin Grittner
On Mon, Apr 18, 2016 at 8:50 AM, Tom Lane wrote: > Surely there was another way to get a similar end result without > mucking with things at the level of BufferGetPage. To get the feature that some customers have been demanding, a check has to be made somewhere near where

Re: [HACKERS] snapshot too old, configured by time

2016-04-18 Thread Alvaro Herrera
Tom Lane wrote: > Alvaro Herrera writes: > > I disagree. A developer that sees an unadorned BufferGetPage() call > > doesn't stop to think twice about whether they need to add a snapshot > > test. Many reviewers will miss the necessary addition also. A > > developer

Re: [HACKERS] snapshot too old, configured by time

2016-04-18 Thread Kevin Grittner
On Mon, Apr 18, 2016 at 8:41 AM, Tom Lane wrote: > Kevin Grittner writes: >> On Sun, Apr 17, 2016 at 10:38 PM, Alvaro Herrera >> wrote: >>> I understand the backpatching pain argument, but my opinion was the >>> contrary of yours

Re: [HACKERS] snapshot too old, configured by time

2016-04-18 Thread Robert Haas
On Sun, Apr 17, 2016 at 6:15 PM, Tom Lane wrote: > After struggling with back-patching a GIN bug fix, I wish to offer up the > considered opinion that this was an impressively bad idea. It's inserted > 450 or so pain points for back-patching, which we will have to deal with >

Re: [HACKERS] snapshot too old, configured by time

2016-04-18 Thread Tom Lane
Alvaro Herrera writes: > I disagree. A developer that sees an unadorned BufferGetPage() call > doesn't stop to think twice about whether they need to add a snapshot > test. Many reviewers will miss the necessary addition also. A > developer that sees

Re: [HACKERS] snapshot too old, configured by time

2016-04-18 Thread Tom Lane
Kevin Grittner writes: > On Sun, Apr 17, 2016 at 10:38 PM, Alvaro Herrera > wrote: >> I understand the backpatching pain argument, but my opinion was the >> contrary of yours even so. > The other possibility would be to backpatch the no-op patch

Re: [HACKERS] snapshot too old, configured by time

2016-04-18 Thread Kevin Grittner
On Sun, Apr 17, 2016 at 10:38 PM, Alvaro Herrera wrote: > Tom Lane wrote: > >> After struggling with back-patching a GIN bug fix, I wish to offer up the >> considered opinion that this was an impressively bad idea. It's inserted >> 450 or so pain points for

Re: [HACKERS] snapshot too old, configured by time

2016-04-17 Thread Alvaro Herrera
Tom Lane wrote: > After struggling with back-patching a GIN bug fix, I wish to offer up the > considered opinion that this was an impressively bad idea. It's inserted > 450 or so pain points for back-patching, which we will have to deal with > for the next five years. Moreover, I do not believe

Re: [HACKERS] snapshot too old, configured by time

2016-04-17 Thread Michael Paquier
On Mon, Apr 18, 2016 at 9:52 AM, Kevin Grittner wrote: > On Sun, Apr 17, 2016 at 5:15 PM, Tom Lane wrote: >> Kevin Grittner writes: >>> On Wed, Mar 30, 2016 at 3:26 PM, Alvaro Herrera> >>> wrote: Kevin

Re: [HACKERS] snapshot too old, configured by time

2016-04-17 Thread Kevin Grittner
On Sun, Apr 17, 2016 at 5:15 PM, Tom Lane wrote: > Kevin Grittner writes: >> On Wed, Mar 30, 2016 at 3:26 PM, Alvaro Herrera> >> wrote: >>> Kevin Grittner wrote: On Wed, Mar 30, 2016 at 2:22 PM, Alvaro Herrera

Re: [HACKERS] snapshot too old, configured by time

2016-04-17 Thread Tom Lane
Kevin Grittner writes: > On Wed, Mar 30, 2016 at 3:26 PM, Alvaro Herrera > wrote: >> Kevin Grittner wrote: >>> On Wed, Mar 30, 2016 at 2:22 PM, Alvaro Herrera >>> wrote: >>> I said that we should change BufferGetPage into

Re: [HACKERS] snapshot too old, configured by time

2016-04-08 Thread David Steele
On 4/8/16 4:30 PM, Peter Geoghegan wrote: > On Fri, Apr 8, 2016 at 12:55 PM, Kevin Grittner wrote: >> Sadly, I forgot to include the reviewer information when writing the >> commit messages. :-( > > Oh well. I'm just glad we got the patch over the line. I think that > there

Re: [HACKERS] snapshot too old, configured by time

2016-04-08 Thread Peter Geoghegan
On Fri, Apr 8, 2016 at 12:55 PM, Kevin Grittner wrote: > Sadly, I forgot to include the reviewer information when writing the > commit messages. :-( Oh well. I'm just glad we got the patch over the line. I think that there are some types of users that will very significantly

Re: [HACKERS] snapshot too old, configured by time

2016-04-08 Thread Kevin Grittner
On Thu, Apr 7, 2016 at 6:12 PM, Peter Geoghegan wrote: > I think that there is a good argument in favor of this patch that you > may have failed to make yourself, which is: it limits bloat in a way > that's analogous to how RecentGlobalDataXmin can do so for logical > decoding

Re: [HACKERS] snapshot too old, configured by time

2016-04-07 Thread Peter Geoghegan
On Thu, Apr 7, 2016 at 3:56 PM, Kevin Grittner wrote: >> Unfortunately, this does stop recycling from >> happening early for B-Tree pages, even though that's probably safe in >> principle. This is probably not so bad -- it just needs to be >> considered when reviewing this

Re: [HACKERS] snapshot too old, configured by time

2016-04-07 Thread Kevin Grittner
On Mon, Apr 4, 2016 at 9:15 PM, Peter Geoghegan wrote: > The patch does this: > >> --- a/src/backend/access/heap/pruneheap.c >> +++ b/src/backend/access/heap/pruneheap.c >> @@ -92,12 +92,21 @@ heap_page_prune_opt(Relation relation, Buffer buffer) >> * need to use the

Re: [HACKERS] snapshot too old, configured by time

2016-04-07 Thread Kevin Grittner
On Sun, Apr 3, 2016 at 4:09 PM, Jeff Janes wrote: > On Wed, Mar 30, 2016 at 12:34 PM, Kevin Grittner wrote: >> On Sat, Mar 19, 2016 at 1:27 AM, Jeff Janes wrote: >>> I set the value to 1min. >>> >>> I set up a test like this: >>>

Re: [HACKERS] snapshot too old, configured by time

2016-04-07 Thread Jeff Janes
On Mon, Apr 4, 2016 at 8:38 PM, Peter Geoghegan wrote: > On Sun, Apr 3, 2016 at 2:09 PM, Jeff Janes wrote: >> Also, HOT-cleanup should stop the bloat increase once the snapshot >> crosses the old_snapshot_threshold without even needing to wait until >> the

Re: [HACKERS] snapshot too old, configured by time

2016-04-04 Thread Peter Geoghegan
On Sun, Apr 3, 2016 at 2:09 PM, Jeff Janes wrote: > Also, HOT-cleanup should stop the bloat increase once the snapshot > crosses the old_snapshot_threshold without even needing to wait until > the next autovac runs. > > Does the code intentionally only work for manual

Re: [HACKERS] snapshot too old, configured by time

2016-04-04 Thread Peter Geoghegan
On Wed, Mar 30, 2016 at 12:46 PM, Kevin Grittner wrote: > On Wed, Mar 30, 2016 at 2:29 PM, Peter Geoghegan wrote: > >> [Does the patch allow dangling page pointers?] > >> Again, I don't want to prejudice anyone against your patch, which I >> haven't read. > >

Re: [HACKERS] snapshot too old, configured by time

2016-04-03 Thread Jeff Janes
On Wed, Mar 30, 2016 at 12:34 PM, Kevin Grittner wrote: > On Sat, Mar 19, 2016 at 1:27 AM, Jeff Janes wrote: > >> I'm not sure if this is operating as expected. >> >> I set the value to 1min. >> >> I set up a test like this: >> >> pgbench -i >> >> pgbench

Re: [HACKERS] snapshot too old, configured by time

2016-04-02 Thread Kevin Grittner
On Sat, Apr 2, 2016 at 7:12 AM, Michael Paquier wrote: > On Fri, Apr 1, 2016 at 11:45 PM, Alvaro Herrera > wrote: >> Kevin Grittner wrote: >> >>> Attached is what I think you're talking about for the first patch. >>> AFAICS this should

Re: [HACKERS] snapshot too old, configured by time

2016-04-02 Thread Michael Paquier
On Fri, Apr 1, 2016 at 11:45 PM, Alvaro Herrera wrote: > Kevin Grittner wrote: > >> Attached is what I think you're talking about for the first patch. >> AFAICS this should generate identical executable code to unpatched. >> Then the patch to actually implement the

Re: [HACKERS] snapshot too old, configured by time

2016-04-01 Thread Alvaro Herrera
Kevin Grittner wrote: > Attached is what I think you're talking about for the first patch. > AFAICS this should generate identical executable code to unpatched. > Then the patch to actually implement the feature would, instead > of adding 30-some lines with TestForOldSnapshot() would implement >

Re: [HACKERS] snapshot too old, configured by time

2016-03-31 Thread Kevin Grittner
On Wed, Mar 30, 2016 at 9:19 PM, Alvaro Herrera wrote: > Michael Paquier wrote: > >> Just a note: I began looking at the tests, but finished looking at the >> patch entirely at the end by curiosity. Regarding the integration of >> this patch for 9.6, I think that bumping

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Alvaro Herrera
Michael Paquier wrote: > Just a note: I began looking at the tests, but finished looking at the > patch entirely at the end by curiosity. Regarding the integration of > this patch for 9.6, I think that bumping that to 9.7 would be wiser > because the patch needs to be re-written largely, and

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Michael Paquier
On Thu, Mar 31, 2016 at 5:09 AM, Kevin Grittner wrote: > On Wed, Mar 30, 2016 at 2:22 PM, Alvaro Herrera > wrote: >> I understand the invasiveness argument, but to me the danger of >> introducing new bugs trumps that. The problem is not the current

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Kevin Grittner
On Thu, Mar 24, 2016 at 2:24 AM, Michael Paquier wrote: > I have been looking at 4a, the test module, and things are looking > good IMO. Something that I think would be adapted would be to define > the options for isolation tests in a variable, like ISOLATION_OPTS to >

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Alvaro Herrera
Kevin Grittner wrote: > On Wed, Mar 30, 2016 at 2:22 PM, Alvaro Herrera > wrote: > > 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

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Kevin Grittner
On Wed, Mar 30, 2016 at 2:22 PM, Alvaro Herrera 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 >

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Kevin Grittner
On Wed, Mar 30, 2016 at 2:34 PM, Kevin Grittner wrote: > A connection should not get the > error just because it is using a snapshot that tries to look at > data that might be wrong, and the connection holding the long-lived > snapshot doesn't do that until it awakes from the

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Kevin Grittner
On Wed, Mar 30, 2016 at 2:29 PM, Peter Geoghegan wrote: > [Does the patch allow dangling page pointers?] > Again, I don't want to prejudice anyone against your patch, which I > haven't read. I don't believe that the way the patch does its business opens any new vulnerabilities

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Kevin Grittner
On Sat, Mar 19, 2016 at 1:27 AM, Jeff Janes wrote: > I'm not sure if this is operating as expected. > > I set the value to 1min. > > I set up a test like this: > > pgbench -i > > pgbench -c4 -j4 -T 3600 & > > ### watch the size of branches table > while (true) ; do psql -c

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Peter Geoghegan
On Wed, Mar 30, 2016 at 12:21 PM, Peter Geoghegan wrote: > Well, amcheck is a tool that in essence makes sure that B-Trees look > structurally sound, and respect invariants like having every item on > each page in logical order. That could catch a bug of the kind I just >

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Alvaro Herrera
Kevin Grittner wrote: > On Wed, Mar 30, 2016 at 11:37 AM, Tom Lane wrote: > > Alvaro Herrera writes: > >> I think a safer proposition would be to replace all current > >> BufferGetPage() calls (there are about 500) by adding the necessary > >>

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Peter Geoghegan
On Wed, Mar 30, 2016 at 11:53 AM, Kevin Grittner wrote: > When the initial "proof of concept" patch was tested by the > customer, it was not effective due to issues related to what you > raise. Autovacuum workers were blocking due to the page pins for > scans using these old

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Kevin Grittner
On Sun, Mar 20, 2016 at 6:25 PM, Peter Geoghegan wrote: > I haven't read the patch, but I wonder: What are the implications here > for B-Tree page recycling by VACUUM? > Offhand, I imagine that there'd be some special considerations. Why is > it okay that an index scan could

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Kevin Grittner
On Wed, Mar 30, 2016 at 11:37 AM, Tom Lane wrote: > Alvaro Herrera writes: >> I think a safer proposition would be to replace all current >> BufferGetPage() calls (there are about 500) by adding the necessary >> arguments: buffer, snapshot, rel, and

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Tom Lane
Alvaro Herrera writes: > I think a safer proposition would be to replace all current > BufferGetPage() calls (there are about 500) by adding the necessary > arguments: buffer, snapshot, rel, and an integer "flags". All this > without adding the feature. Then a

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Alvaro Herrera
Michael Paquier wrote: > page = BufferGetPage(buf); > + TestForOldSnapshot(scan->xs_snapshot, rel, page); > This is a sequence repeated many times in this patch, a new routine, > say BufferGetPageExtended with a uint8 flag, one flag being used to > test old snapshots would be more adapted.

Re: [HACKERS] snapshot too old, configured by time

2016-03-30 Thread Kevin Grittner
On Tue, Mar 29, 2016 at 8:58 AM, David Steele wrote: > We're getting to the end of the CF now. Do you know when you'll have an > updated patch ready? I am working on it right now. Hopefully I can get it all sorted today. -- Kevin Grittner EDB:

Re: [HACKERS] snapshot too old, configured by time

2016-03-29 Thread David Steele
Hi Kevin, On 3/21/16 4:05 PM, Kevin Grittner wrote: Thanks to all for the feedback; I will try to respond later this week. First I'm trying to get my reviews for other patches posted. We're getting to the end of the CF now. Do you know when you'll have an updated patch ready? Thanks, --

Re: [HACKERS] snapshot too old, configured by time

2016-03-24 Thread Michael Paquier
On Tue, Mar 22, 2016 at 5:05 AM, Kevin Grittner wrote: > Thanks to all for the feedback; I will try to respond later this > week. First I'm trying to get my reviews for other patches posted. I have been looking at 4a, the test module, and things are looking good IMO.

Re: [HACKERS] snapshot too old, configured by time

2016-03-21 Thread Kevin Grittner
Thanks to all for the feedback; I will try to respond later this week. First I'm trying to get my reviews for other patches posted. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To

Re: [HACKERS] snapshot too old, configured by time

2016-03-21 Thread Thom Brown
On 17 March 2016 at 21:15, Kevin Grittner wrote: > New patch just to merge in recent commits -- it was starting to > show some bit-rot. Tests folded in with main patch. In session 1, I've run: # begin transaction isolation level repeatable read ; BEGIN *# declare stuff

Re: [HACKERS] snapshot too old, configured by time

2016-03-20 Thread Peter Geoghegan
On Sun, Mar 20, 2016 at 4:25 PM, Peter Geoghegan wrote: > I worry that something weird could happen there. For example, perhaps > the page LSN on what is actually a newly recycled page could be set > such that the backend following a stale right spuriously raises a > "snapshot

Re: [HACKERS] snapshot too old, configured by time

2016-03-20 Thread Peter Geoghegan
On Thu, Mar 17, 2016 at 2:15 PM, Kevin Grittner wrote: > New patch just to merge in recent commits -- it was starting to > show some bit-rot. Tests folded in with main patch. I haven't read the patch, but I wonder: What are the implications here for B-Tree page recycling by

Re: [HACKERS] snapshot too old, configured by time

2016-03-19 Thread Kevin Grittner
New patch just to merge in recent commits -- it was starting to show some bit-rot. Tests folded in with main patch. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company snapshot-too-old-v5.patch Description: invalid/octet-stream -- Sent via pgsql-hackers

Re: [HACKERS] snapshot too old, configured by time

2016-03-19 Thread Jeff Janes
On Thu, Mar 17, 2016 at 2:15 PM, Kevin Grittner wrote: > New patch just to merge in recent commits -- it was starting to > show some bit-rot. Tests folded in with main patch. I'm not sure if this is operating as expected. I set the value to 1min. I set up a test like this:

Re: [HACKERS] snapshot too old, configured by time

2016-03-11 Thread Michael Paquier
On Fri, Mar 11, 2016 at 2:35 PM, Robert Haas wrote: > On Thu, Mar 3, 2016 at 2:40 PM, Kevin Grittner wrote: >> Thanks for the tips. Attached is a minimal set of isolation tests. >> I can expand on it if needed, but wanted: >> >> (1) to confirm that this

Re: [HACKERS] snapshot too old, configured by time

2016-03-11 Thread Robert Haas
On Thu, Mar 3, 2016 at 2:40 PM, Kevin Grittner wrote: > Thanks for the tips. Attached is a minimal set of isolation tests. > I can expand on it if needed, but wanted: > > (1) to confirm that this is the right way to do this, and > > (2) how long people were willing to tolerate

Re: [HACKERS] snapshot too old, configured by time

2016-03-03 Thread Kevin Grittner
On Tue, Mar 1, 2016 at 12:58 AM, Michael Paquier wrote: > On Tue, Mar 1, 2016 at 9:35 AM, Andres Freund wrote: >> On 2016-02-29 18:30:27 -0600, Kevin Grittner wrote: >>> Basically, a connection needs to remain open and interleave >>> commands with

Re: [HACKERS] snapshot too old, configured by time

2016-02-29 Thread Michael Paquier
On Tue, Mar 1, 2016 at 9:35 AM, Andres Freund wrote: > On 2016-02-29 18:30:27 -0600, Kevin Grittner wrote: >> Basically, a connection needs to remain open and interleave >> commands with other connections, which the isolation tester does >> just fine; but it needs to do that

Re: [HACKERS] snapshot too old, configured by time

2016-02-29 Thread Andres Freund
On 2016-02-29 18:30:27 -0600, Kevin Grittner wrote: > Basically, a connection needs to remain open and interleave > commands with other connections, which the isolation tester does > just fine; but it needs to do that using a custom postgresql.conf > file, which TAP does just fine. I haven't been

Re: [HACKERS] snapshot too old, configured by time

2016-02-29 Thread Kevin Grittner
On Fri, Jan 8, 2016 at 5:22 PM, Alvaro Herrera wrote: >> People have said that issuing SQL commands directly from a TAP test >> via DBD::Pg is not acceptable for a core feature, and (despite >> assertions to the contrary) I see no way to test this feature with >>

Re: [HACKERS] snapshot too old, configured by time

2016-02-01 Thread Alvaro Herrera
Kevin Grittner wrote: > > There has been a review but no replies for more than 1 month. Returned > > with feedback? > > I do intend to post another version of the patch to tweak the > calculations again, after I can get a patch in to expand the > testing capabilities to allow an acceptable way

Re: [HACKERS] snapshot too old, configured by time

2016-01-08 Thread Alvaro Herrera
Kevin Grittner wrote: > People have said that issuing SQL commands directly from a TAP test > via DBD::Pg is not acceptable for a core feature, and (despite > assertions to the contrary) I see no way to test this feature with > existing testing mechanisms. The bigger set of work here, if we >

Re: [HACKERS] snapshot too old, configured by time

2015-12-03 Thread Andres Freund
On 2015-12-02 14:48:24 -0600, Kevin Grittner wrote: > On Wed, Dec 2, 2015 at 12:39 AM, Michael Paquier > wrote: > > On Mon, Nov 9, 2015 at 8:07 AM, Steve Singer wrote: > > >> In snapmgr.c > >> > >> > >> + * XXX: If we can trust a read of an int64

Re: [HACKERS] snapshot too old, configured by time

2015-12-02 Thread Kevin Grittner
On Wed, Dec 2, 2015 at 12:39 AM, Michael Paquier wrote: > On Mon, Nov 9, 2015 at 8:07 AM, Steve Singer wrote: >> In snapmgr.c >> >> >> + * XXX: If we can trust a read of an int64 value to be atomic, we can skip >> the >> + * spinlock here. >> + */

  1   2   3   >