Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)

On Thu, Jan 19, 2023 at 6:18 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> The patch does not apply on top of HEAD as in [1], please post a rebased patch:
> === Applying patches on top of PostgreSQL commit ID
> 4f74f5641d53559ec44e74d5bf552e167fdd5d20 ===
> === applying patch
> ./v49-0003-Add-system-view-tracking-IO-ops-per-backend-type.patch
> ....
> patching file src/test/regress/expected/rules.out
> Hunk #1 FAILED at 1876.
> 1 out of 1 hunk FAILED -- saving rejects to file
> src/test/regress/expected/rules.out.rej
>
> [1] - http://cfbot.cputube.org/patch_41_3272.log

Yes, it conflicted with 47bb9db75996232. rebased v50 is attached.

On Tue, Jan 17, 2023 at 5:00 PM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
> > > > > +-- Change the tablespace so that the table is rewritten directly, then SELECT
> > > > > +-- from it to cause it to be read back into shared buffers.
> > > > > +SET allow_in_place_tablespaces = true;
> > > > > +CREATE TABLESPACE regress_io_stats_tblspc LOCATION '';
> > > >
> > > > Perhaps worth doing this in tablespace.sql, to avoid the additional
> > > > checkpoints done as part of CREATE/DROP TABLESPACE?
> > > >
> > > > Or, at least combine this with the CHECKPOINTs above?
> > >
> > > I see a checkpoint is requested when dropping the tablespace if not all
> > > the files in it are deleted. It seems like if the DROP TABLE for the
> > > permanent table is before the explicit checkpoints in the test, then the
> > > DROP TABLESPACE will not cause an additional checkpoint.
> >
> > Unfortunately, that's not how it works :(. See the comment above mdunlink():
> >
> > > * For regular relations, we don't unlink the first segment file of the rel,
> > > * but just truncate it to zero length, and record a request to unlink it after
> > > * the next checkpoint. Additional segments can be unlinked immediately,
> > > * however. Leaving the empty file in place prevents that relfilenumber
> > > * from being reused. The scenario this protects us from is:
> > > ...
> >
> >
> > > Is this what you are suggesting? Dropping the temporary table should not
> > > have an effect on this.
> >
> > I was wondering about simply moving that portion of the test to
> > tablespace.sql, where we already created a tablespace.
> >
> >
> > An alternative would be to propose splitting tablespace.sql into one portion
> > running at the start of parallel_schedule, and one at the end. Historically,
> > we needed tablespace.sql to be optional due to causing problems when
> > replicating to another instance on the same machine, but now we have
> > allow_in_place_tablespaces.
>
> It seems like the best way would be to split up the tablespace test file
> as you suggested and drop the tablespace at the end of the regression
> test suite. There could be other tests that could use a tablespace.
> Though what I wrote is kind of tablespace test coverage, if this
> rewriting behavior no longer happened when doing alter table set
> tablespace, we would want to come up with a new test which exercised
> that code to count those IO stats, not simply delete it from the
> tablespace tests.

I have added a patch to the set which creates the regress_tblspace
(formerly created in tablespace.sq1) in test_setup.sql. I then moved the
tablespace test to the end of the parallel schedule so that my test (and
others) could use the regress_tblspace.

I modified some of the tablespace.sql tests to be more specific in terms
of the objects they are looking for so that tests using the tablespace
are not forced to drop all of the objects they make in the tablespace.

Note that I did not proactively change all tests in tablespace.sql that
may fail in this way -- only those that failed because of the tables I
created (and did not drop) from regress_tblspace.

- Melanie