From: | Kirill Bychik <kirill(dot)bychik(at)gmail(dot)com> |
---|---|
To: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Cc: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com> |
Subject: | Re: WAL usage calculation patch |
Date: | 2020-03-17 19:27:05 |
Message-ID: | CAB-hujrd+8kb5o+k1dy=r3oeuxnWO_PuaQiOiPHQHc8unH5ywg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> > Please feel free to work on any extension of this patch idea. I lack
> > both time and knowledge to do it all by myself.
>
>
> I'm adding a 3rd patch on top of yours to expose the new WAL counters in
> pg_stat_database, for vacuum and autovacuum. I'm not really enthiusiastic with
> this approach but I didn't find better, and maybe this will raise some better
> ideas. The only sure thing is that we're not going to add a bunch of new
> fields in pg_stat_all_tables anyway.
>
> We can also drop this 3rd patch entirely if no one's happy about it without
> impacting the first two.
No objections about 3rd on my side, unless we miss the CF completely.
As for the code, I believe:
+ walusage.wal_records = pgWalUsage.wal_records -
+ walusage_start.wal_records;
+ walusage.wal_fp_records = pgWalUsage.wal_fp_records -
+ walusage_start.wal_fp_records;
+ walusage.wal_bytes = pgWalUsage.wal_bytes - walusage_start.wal_bytes;
Could be done much simpler via the utility:
WalUsageAccumDiff(walusage, pgWalUsage, walusage_start);
On a side note, I agree API to the buf/wal usage is far from perfect.
> > Test had been reworked, and I believe it should be stable now, the
> > part which checks WAL is written and there is a correlation between
> > affected rows and WAL records. I still have no idea how to test
> > full-page writes against regular updates, it seems very unstable.
> > Please share ideas if any.
>
>
> I just reviewed the patches, and it globally looks good to me. The way to
> detect full page images looks sensible, but I'm really not familiar with that
> code so additional review would be useful.
>
> I noticed that the new wal_write_fp_records field in pg_stat_statements wasn't
> used in the test. Since I have to add all the patches to make the cfbot happy,
> I slightly adapted the tests to reference the fp column too. There was also a
> minor issue in the documentation, as wal_records and wal_bytes were copy/pasted
> twice while wal_write_fp_records wasn't documented, so I also changed it.
>
> Let me know if you're ok with those changes.
Sorry for not getting wal_fp_usage into the docs, my fault.
As for the tests, please get somebody else to review this. I strongly
believe checking full page writes here could be a source of
instability.
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2020-03-17 19:30:14 | Re: [PATCH] Btree BackwardScan race condition on Standby during VACUUM |
Previous Message | Alvaro Herrera | 2020-03-17 19:24:48 | Re: logical copy_replication_slot issues |