logtape.c stats don't account for unused "prefetched" block numbers

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
33 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Jeff Davis-8
On Mon, 2020-09-14 at 19:29 -0700, Peter Geoghegan wrote:
> Let's assume that we'll teach LogicalTapeSetBlocks() to use
> nBlocksWritten in place of nBlocksAllocated in all cases, as now
> seems
> likely. Rather than asserting "nBlocksWritten == nBlocksAllocated"
> inside LogicalTapeSetBlocks() (as I suggested earlier at one point),
> we could instead teach LogicalTapeSetBlocks() to iterate through each
> tape from the tapeset and make sure each tape has no writes buffered
> (so everything must be flushed). We could add a loop that would only
> be used on assert-enabled builds.

Sounds reasonable.

> You suggested this yourself, Jeff (my suggestion about the assertion
> is just an expansion on your suggestion from earlier). This all seems
> like a good idea to me. Can you write a patch that adjusts
> LogicalTapeSetBlocks() along these lines? Hopefully the assertion
> loop
> thing won't reveal some other problem with this plan.

Sure. Will backporting either patch into REL_13_STABLE now interfere
with RC1 release in any way?

Regards,
        Jeff Davis




Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Peter Geoghegan-4
On Mon, Sep 14, 2020 at 8:07 PM Jeff Davis <[hidden email]> wrote:
> Sure. Will backporting either patch into REL_13_STABLE now interfere
> with RC1 release in any way?

The RMT will discuss this.

It would help if there was a patch ready to go.

Thanks
--
Peter Geoghegan


Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Jeff Davis-8
On Mon, 2020-09-14 at 20:48 -0700, Peter Geoghegan wrote:
> On Mon, Sep 14, 2020 at 8:07 PM Jeff Davis <[hidden email]> wrote:
> > Sure. Will backporting either patch into REL_13_STABLE now
> > interfere
> > with RC1 release in any way?
>
> The RMT will discuss this.
>
> It would help if there was a patch ready to go.

Attached. HashAgg seems to accurately reflect the filesize, as does
Sort.

Regards,
        Jeff Davis


logtape-nblockswritten.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Jeff Davis-8
In reply to this post by Peter Geoghegan-4
On Thu, 2020-09-10 at 18:42 -0700, Peter Geoghegan wrote:
> We still need to put the reliance on ltsWriteBlock() allocating many
> blocks before they've been logically written on some kind of formal
> footing for Postgres 13 -- it is now possible that an all-zero block
> will be left behind even after we're done writing and have flushed
> all
> temp buffers, which is a new thing.

Is the current direction of this thread (i.e. the two posted patches)
addressing your concern here?

Regards,
        Jeff Davis




Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Peter Geoghegan-4
On Mon, Sep 14, 2020 at 11:52 PM Jeff Davis <[hidden email]> wrote:

> On Thu, 2020-09-10 at 18:42 -0700, Peter Geoghegan wrote:
> > We still need to put the reliance on ltsWriteBlock() allocating many
> > blocks before they've been logically written on some kind of formal
> > footing for Postgres 13 -- it is now possible that an all-zero block
> > will be left behind even after we're done writing and have flushed
> > all
> > temp buffers, which is a new thing.
>
> Is the current direction of this thread (i.e. the two posted patches)
> addressing your concern here?

Yes.

--
Peter Geoghegan


Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Peter Geoghegan-4
In reply to this post by Peter Geoghegan-4
On Mon, Sep 14, 2020 at 8:48 PM Peter Geoghegan <[hidden email]> wrote:
> On Mon, Sep 14, 2020 at 8:07 PM Jeff Davis <[hidden email]> wrote:
> > Sure. Will backporting either patch into REL_13_STABLE now interfere
> > with RC1 release in any way?
>
> The RMT will discuss this.

It is okay to skip RC1 and commit the patch/patches for 13 itself.
Please wait until after Tom has pushed the rc1 tag. This will probably
happen tomorrow.

--
Peter Geoghegan


Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Tom Lane-2
Peter Geoghegan <[hidden email]> writes:
> On Mon, Sep 14, 2020 at 8:48 PM Peter Geoghegan <[hidden email]> wrote:
>> On Mon, Sep 14, 2020 at 8:07 PM Jeff Davis <[hidden email]> wrote:
>>> Sure. Will backporting either patch into REL_13_STABLE now interfere
>>> with RC1 release in any way?

> It is okay to skip RC1 and commit the patch/patches for 13 itself.
> Please wait until after Tom has pushed the rc1 tag. This will probably
> happen tomorrow.

I plan to tag rc1 in around six hours, ~2200UTC today, barring
trouble reports from packagers (none so far).  Feel free to push your
patch once the tag appears.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Peter Geoghegan-4
In reply to this post by Jeff Davis-8
On Mon, Sep 14, 2020 at 11:44 PM Jeff Davis <[hidden email]> wrote:
> Attached. HashAgg seems to accurately reflect the filesize, as does
> Sort.

For the avoidance of doubt: I think that this is the right way to go,
and that it should be committed shortly, before we stamp 13.0. The
same goes for hashagg-release-write-buffers.patch.

Thanks
--
Peter Geoghegan


Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Tom Lane-2
In reply to this post by Tom Lane-2
I wrote:
> I plan to tag rc1 in around six hours, ~2200UTC today, barring
> trouble reports from packagers (none so far).  Feel free to push your
> patch once the tag appears.

The tag is applied, though for some reason the pgsql-committers auto
e-mail about new tags hasn't been working lately.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Peter Geoghegan-4
On Tue, Sep 15, 2020 at 3:02 PM Tom Lane <[hidden email]> wrote:
> The tag is applied, though for some reason the pgsql-committers auto
> e-mail about new tags hasn't been working lately.

Thanks. FWIW I did get the automated email shortly after you sent this email.

--
Peter Geoghegan


Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Tom Lane-2
Peter Geoghegan <[hidden email]> writes:
> On Tue, Sep 15, 2020 at 3:02 PM Tom Lane <[hidden email]> wrote:
>> The tag is applied, though for some reason the pgsql-committers auto
>> e-mail about new tags hasn't been working lately.

> Thanks. FWIW I did get the automated email shortly after you sent this email.

Yeah, it did show up here too, about an hour after I pushed the tag.
The last several taggings have been delayed similarly, and I think
at least one never was reported at all.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Alvaro Herrera-9
On 2020-Sep-15, Tom Lane wrote:

> Peter Geoghegan <[hidden email]> writes:
> > On Tue, Sep 15, 2020 at 3:02 PM Tom Lane <[hidden email]> wrote:
> >> The tag is applied, though for some reason the pgsql-committers auto
> >> e-mail about new tags hasn't been working lately.
>
> > Thanks. FWIW I did get the automated email shortly after you sent this email.
>
> Yeah, it did show up here too, about an hour after I pushed the tag.
> The last several taggings have been delayed similarly, and I think
> at least one never was reported at all.

I approved it about half an hour after it got in the moderation queue.

They get moderated because the [hidden email] address which
appears as sender is not subscribed to any list.  I also added that
address to the whitelist now, but whether that's a great fix in the long
run is debatable.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply | Threaded
Open this post in threaded view
|

Re: logtape.c stats don't account for unused "prefetched" block numbers

Tom Lane-2
Alvaro Herrera <[hidden email]> writes:
> They get moderated because the [hidden email] address which
> appears as sender is not subscribed to any list.

Ah-hah.

> I also added that
> address to the whitelist now, but whether that's a great fix in the long
> run is debatable.

Can/should we change the address that originates such messages?

                        regards, tom lane


12