On Tue, Mar 7, 2023 at 12:48 PM Michael Paquier <mich...@paquier.xyz> wrote: > > On Tue, Mar 07, 2023 at 03:49:02PM +0900, Kyotaro Horiguchi wrote: > > Ah. Yes, that expansion sounds sensible. > > Okay, so, based on this idea, I have hacked on this stuff and finish > with the attached that shows block data if it exists, as well as FPI > stuff if any. bimg_info is showed as a text[] for its flags.
+1. > I guess that I'd better add a test that shows correctly a record with > some block data attached to it, on top of the existing one for FPIs.. > Any suggestions? Perhaps just a heap/heap2 record? > > Thoughts? That would be a lot better. Not just the test, but also the documentation can have it. Simple way to generate such a record (both block data and FPI) is to just change the wal_level to logical in walinspect.conf [1], see code around REGBUF_KEEP_DATA and RelationIsLogicallyLogged in heapam.c I had the following comments and fixed them in the attached v2 patch: 1. Still a trace of pg_get_wal_fpi_info in docs, removed it. 2. Used int4 instead of int for fpilen just to be in sync with fpi_length of pg_get_wal_record_info. 3. Changed to be consistent and use just FPI or "F/full page". /* FPI flags */ /* No full page image, so store NULLs for all its fields */ /* Full-page image */ /* Full page exists, so let's save it. */ * and end LSNs. This produces information about the full page images with * to a record. Decompression is applied to the full-page images, if 4. I think we need to free raw_data, raw_page and flags as we loop over multiple blocks (XLR_MAX_BLOCK_ID) and will leak memory which can be a problem if we have many blocks assocated with a single WAL record. flags = (Datum *) palloc0(sizeof(Datum) * bitcnt); Also, we will leak all CStringGetTextDatum memory in the block_id for loop. Another way is to use and reset temp memory context in the for loop over block_ids. I prefer this approach over multiple pfree()s in block_id for loop. 5. I think it'd be good to say if the FPI is for WAL_VERIFICATION, so I changed it to the following. Feel free to ignore this if you think it's not required. if (blk->apply_image) flags[cnt++] = CStringGetTextDatum("APPLY"); else flags[cnt++] = CStringGetTextDatum("WAL_VERIFICATION"); 6. Did minor wordsmithing. 7. Added test case which shows both block data and fpi in the documentation. 8. Changed wal_level to logical in walinspect.conf to test case with block data. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
v2-0001-Rework-pg_walinspect-to-retrieve-more-block-infor.patch
Description: Binary data