Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-02-03 Thread Thomas Munro
On Mon, Feb 4, 2019 at 1:09 PM Amit Kapila wrote: > On Sun, Feb 3, 2019 at 8:48 PM Andres Freund wrote: > > And one on eelpout, which appears to be unrelated as well: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=eelpout=2019-02-03%2011%3A54%3A13 > None of these failures seem to

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-02-03 Thread Amit Kapila
On Sun, Feb 3, 2019 at 8:48 PM Andres Freund wrote: > > Hi, > > On 2019-02-01 07:14:11 -0800, Andres Freund wrote: > > Here's a version of the patch implementing this approach. I assume this > > solves the FreeBSD issue, but I'm running tests in a loop on Thomas' > > machine. > > I pushed this,

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-02-03 Thread Andres Freund
Hi, On 2019-02-01 07:14:11 -0800, Andres Freund wrote: > Here's a version of the patch implementing this approach. I assume this > solves the FreeBSD issue, but I'm running tests in a loop on Thomas' > machine. I pushed this, and the buildfarm so far is showing more love. There's a failure on

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-02-01 Thread Andres Freund
Hi, On 2019-01-29 12:23:51 -0800, Andres Freund wrote: > On 2019-01-29 11:25:41 -0800, Andres Freund wrote: > > On 2019-01-28 22:37:53 -0500, Tom Lane wrote: > > > Andres Freund writes: > > > > I did that now. I couldn't reproduce it locally, despite a lot of > > > > runs. Looking at the

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-29 Thread Andres Freund
On 2019-01-29 11:25:41 -0800, Andres Freund wrote: > While chatting with Robert about this issue I came across the following > section of code: > > /* >* If the FSM knows nothing of the rel, try the last page > before we >* give up and extend. This

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-29 Thread Andres Freund
On 2019-01-29 11:25:41 -0800, Andres Freund wrote: > On 2019-01-28 22:37:53 -0500, Tom Lane wrote: > > Andres Freund writes: > > > I did that now. I couldn't reproduce it locally, despite a lot of > > > runs. Looking at the buildfarm it looks like the failures were, > > > excluding handfish which

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-29 Thread Andres Freund
On 2019-01-28 22:37:53 -0500, Tom Lane wrote: > Andres Freund writes: > > I did that now. I couldn't reproduce it locally, despite a lot of > > runs. Looking at the buildfarm it looks like the failures were, > > excluding handfish which failed without recognizable symptoms before and > > after,

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-28 Thread Tom Lane
Andres Freund writes: > I did that now. I couldn't reproduce it locally, despite a lot of > runs. Looking at the buildfarm it looks like the failures were, > excluding handfish which failed without recognizable symptoms before and > after, on BSD derived platforms (netbsd, freebsd, OX), which

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-28 Thread Andres Freund
Hi, On 2019-01-28 16:40:36 -0800, Andres Freund wrote: > On 2019-01-28 15:49:33 -0800, Andres Freund wrote: > > On 2019-01-28 18:08:59 -0500, Tom Lane wrote: > > > Andres Freund writes: > > > > I'm inclined to put back the > > > >LockBuffer(buf, BUFFER_LOCK_UNLOCK); > > > >

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-28 Thread Andres Freund
Hi, On 2019-01-28 15:49:33 -0800, Andres Freund wrote: > On 2019-01-28 18:08:59 -0500, Tom Lane wrote: > > Andres Freund writes: > > > I'm inclined to put back the > > >LockBuffer(buf, BUFFER_LOCK_UNLOCK); > > >LockRelationForExtension(onerel, ExclusiveLock); > > >

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-28 Thread Andres Freund
Hi, On 2019-01-28 18:08:59 -0500, Tom Lane wrote: > Andres Freund writes: > > I'm inclined to put back the > >LockBuffer(buf, BUFFER_LOCK_UNLOCK); > >LockRelationForExtension(onerel, ExclusiveLock); > >UnlockRelationForExtension(onerel, ExclusiveLock); > >

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-28 Thread Tom Lane
Andres Freund writes: > I'm inclined to put back the >LockBuffer(buf, BUFFER_LOCK_UNLOCK); >LockRelationForExtension(onerel, ExclusiveLock); >UnlockRelationForExtension(onerel, ExclusiveLock); >LockBufferForCleanup(buf); >if

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-28 Thread Andres Freund
Hi, On 2019-01-28 14:10:55 -0800, Andres Freund wrote: > So, I'd pushed the latest version. And longfin came back with an > interesting error: > > ERROR: page 135 of relation "pg_class" should be empty but is not > > The only way I can currently imagine this happening is that there's a >

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-28 Thread Andres Freund
Hi, So, I'd pushed the latest version. And longfin came back with an interesting error: ERROR: page 135 of relation "pg_class" should be empty but is not The only way I can currently imagine this happening is that there's a concurrent vacuum that discovers the page is empty, enters it into the

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-28 Thread Andres Freund
Hi, On 2019-01-15 17:35:21 -0500, Tom Lane wrote: > Andres Freund writes: > >> Looking at the surrounding code made me wonder about the wisdom of > >> entering empty pages as all-visible and all-frozen into the VM. That'll > >> mean we'll never re-discover them on a primary, after promotion.

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2019-01-15 Thread Andres Freund
Hi, On 2018-12-20 15:04:11 -0800, Andres Freund wrote: > On 2018-12-19 16:56:36 -0800, Andres Freund wrote: > > On 2018-12-19 19:39:33 -0500, Tom Lane wrote: > > > Robert Haas writes: > > > > On Wed, Dec 19, 2018 at 5:37 PM Andres Freund > > > > wrote: > > > >> What's gained by the logic of

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2018-12-20 Thread Andres Freund
Hi, On 2018-12-19 16:56:36 -0800, Andres Freund wrote: > On 2018-12-19 19:39:33 -0500, Tom Lane wrote: > > Robert Haas writes: > > > On Wed, Dec 19, 2018 at 5:37 PM Andres Freund wrote: > > >> What's gained by the logic of emitting that warning in VACUUM after a > > >> crash? I don't really see

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2018-12-19 Thread Andres Freund
Hi, On 2018-12-19 19:39:33 -0500, Tom Lane wrote: > Robert Haas writes: > > On Wed, Dec 19, 2018 at 5:37 PM Andres Freund wrote: > >> What's gained by the logic of emitting that warning in VACUUM after a > >> crash? I don't really see any robustness advantages in it. > > > I don't know. I am

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2018-12-19 Thread Robert Haas
On Wed, Dec 19, 2018 at 5:37 PM Andres Freund wrote: > What's gained by the logic of emitting that warning in VACUUM after a > crash? I don't really see any robustness advantages in it. If the logic > were that we'd never reuse empty pages because they can hide corruption > that normally would

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2018-12-19 Thread Andres Freund
Hi, On 2018-12-19 17:28:17 -0500, Robert Haas wrote: > On Wed, Dec 19, 2018 at 3:39 AM Andres Freund wrote: > > Thinking about whether it's worth to allow to extend that function in an > > extensible manner made me wonder: Is it actually a good idea to > > initialize the page at that point,

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2018-12-19 Thread Robert Haas
On Wed, Dec 19, 2018 at 3:39 AM Andres Freund wrote: > Thinking about whether it's worth to allow to extend that function in an > extensible manner made me wonder: Is it actually a good idea to > initialize the page at that point, including marking it dirty? As far as I can recall, my

Re: Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2018-12-19 Thread Amit Kapila
On Wed, Dec 19, 2018 at 2:09 PM Andres Freund wrote: > > Hi, > > The zheap patchset, even after being based on pluggable storage, > currently has the following condition in RelationAddExtraBlocks(): > if (RelationStorageIsZHeap(relation)) > { >

Why are we PageInit'ing buffers in RelationAddExtraBlocks()?

2018-12-19 Thread Andres Freund
Hi, The zheap patchset, even after being based on pluggable storage, currently has the following condition in RelationAddExtraBlocks(): if (RelationStorageIsZHeap(relation)) { Assert(BufferGetBlockNumber(buffer) != ZHEAP_METAPAGE);