On Thu, Mar 30, 2023 at 2:41 PM Peter Geoghegan wrote:
> There is still an outstanding question around the overhead of
> outputting FPIs and even block data from pg_get_wal_block_info(). At
> one point Melanie suggested that we'd need to do something about that,
> and I tend to agree. Attached
On Thu, Mar 30, 2023 at 2:41 PM Peter Geoghegan wrote:
> pg@regression:5432 [1402115]=# SELECT
> count(*)
> FROM
> pg_get_wal_block_info ('0/10E9D80', '/', true);
> ┌─[ RECORD 1 ]───┐
> │ count │ 17,031,979 │
> └───┴┘
>
> Time: 15235.499 ms (00:15.235)
>
>
On Wed, Mar 29, 2023 at 8:28 PM Bharath Rupireddy
wrote:
> I took a look at v9 and LGTM.
Pushed, thanks.
There is still an outstanding question around the overhead of
outputting FPIs and even block data from pg_get_wal_block_info(). At
one point Melanie suggested that we'd need to do something
On Thu, Mar 30, 2023 at 5:15 AM Peter Geoghegan wrote:
>
> On Wed, Mar 29, 2023 at 12:47 PM Peter Geoghegan wrote:
> > A couple of small tweaks to this appear in the attached revision, v8.
>
> I spent some time on the documentation today, too. Attached is v9,
> which seems pretty close to being
On Wed, Mar 29, 2023 at 12:47 PM Peter Geoghegan wrote:
> A couple of small tweaks to this appear in the attached revision, v8.
I spent some time on the documentation today, too. Attached is v9,
which seems pretty close to being committable. I hope to commit what I
have here (or something very
On Mon, Mar 27, 2023 at 7:40 PM Peter Geoghegan wrote:
> Attached revision v7 adjusts the column order. This is still WIP, but
> gives a good idea of the direction I'm going in.
A couple of small tweaks to this appear in the attached revision, v8.
Now it looks like this:
pg@regression:5432
On Tue, Mar 28, 2023 at 3:34 PM Michael Paquier wrote:
> I disagree with this argument. Personally, I have a *much* better
> experience with textual representation because there is no need to
> cross-check the internals of the code in case you don't remember what
> a given number means in an
On Tue, Mar 28, 2023 at 11:15:17AM -0700, Peter Geoghegan wrote:
> On Mon, Mar 27, 2023 at 7:47 PM Bharath Rupireddy
> wrote:
>> Hm, agreed. Changed in the attached v7-0002 patch. We can as well
>> write a case statement in the create function SQL to output forkname
>> instead forknumber, but I'd
On Mon, Mar 27, 2023 at 7:47 PM Bharath Rupireddy
wrote:
> Hm, agreed. Changed in the attached v7-0002 patch. We can as well
> write a case statement in the create function SQL to output forkname
> instead forknumber, but I'd stop doing that to keep in sync with
> pg_buffercache.
I just don't
On Mon, Mar 27, 2023 at 06:07:09PM -0700, Peter Geoghegan wrote:
> I see a bug on HEAD, following yesterday's commit 0276ae42dd.
>
> GetWALRecordInfo() will now output the value of the fpi_len variable
> before it has actually been set by our call to . So it'll always
> be 0.
Indeed, good
On Tue, Mar 28, 2023 at 5:29 AM Peter Geoghegan wrote:
>
> On Mon, Mar 27, 2023 at 12:42 AM Bharath Rupireddy
> wrote:
> > Thanks. Here's the v6 patch (last patch that I have with me for
> > pg_walinspect) for adding per-record info to pg_get_wal_block_info.
> > Note that I addressed all review
On Mon, Mar 27, 2023 at 4:59 PM Peter Geoghegan wrote:
> Looking at this now, with the intention of committing it for 16.
Attached revision v7 adjusts the column order. This is still WIP, but
gives a good idea of the direction I'm going in.
v7 makes the column output look like this:
On Mon, Mar 27, 2023 at 4:59 PM Peter Geoghegan wrote:
> Looking at this now, with the intention of committing it for 16.
I see a bug on HEAD, following yesterday's commit 0276ae42dd.
GetWALRecordInfo() will now output the value of the fpi_len variable
before it has actually been set by our
On Mon, Mar 27, 2023 at 12:42 AM Bharath Rupireddy
wrote:
> Thanks. Here's the v6 patch (last patch that I have with me for
> pg_walinspect) for adding per-record info to pg_get_wal_block_info.
> Note that I addressed all review comments received so far. Any
> thoughts?
Looking at this now, with
On Sun, Mar 26, 2023 at 8:41 PM Kyotaro Horiguchi
wrote:
> > I'd still put the LSN data before the three OIDs for consistency with
> > the structures, though my opinion does not seem to count much..
>
> I agree with Michael on this point. Also, although it may not be
> significant for SQL, the
On Mon, Mar 27, 2023 at 9:49 AM Michael Paquier wrote:
>
> On Sat, Mar 25, 2023 at 12:12:50PM +0900, Michael Paquier wrote:
> > I don't see any need to move this block of code? This leads to
> > unnecessary diffs, potentially making backpatch a bit harder. Either
> > way is not a big deal,
On Sat, Mar 25, 2023 at 12:12:50PM +0900, Michael Paquier wrote:
> I don't see any need to move this block of code? This leads to
> unnecessary diffs, potentially making backpatch a bit harder. Either
> way is not a big deal, still.. Except for this bit, 0001 looks fine
> by me.
FYI, I have
On Mon, Mar 27, 2023 at 9:11 AM Kyotaro Horiguchi
wrote:
>
> At Sat, 25 Mar 2023 12:12:50 +0900, Michael Paquier
> wrote in
> > On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote:
> > OUT reltablespace oid,
> > OUT reldatabase oid,
> > OUT relfilenode oid,
At Sat, 25 Mar 2023 12:12:50 +0900, Michael Paquier wrote
in
> On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote:
> OUT reltablespace oid,
> OUT reldatabase oid,
> OUT relfilenode oid,
> OUT relblocknumber int8,
> + OUT blockid int2,
> +
On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote:
> Please see the attached v4 patch set addressing all the review comments.
- desc = GetRmgr(XLogRecGetRmid(record));
- id = desc.rm_identify(XLogRecGetInfo(record));
-
- if (id == NULL)
- id = psprintf("UNKNOWN (%x)",
On Mon, Mar 20, 2023 at 8:51 AM Kyotaro Horiguchi
wrote:
>
> + /* Get block references, if any, otherwise continue. */
> + if (!XLogRecHasAnyBlockRefs(xlogreader))
>
> code does". I feel we don't need the a comment there.
Removed.
> This means GetWALBlockInfo
On Wed, Mar 22, 2023 at 5:14 PM Michael Paquier wrote:
> On Wed, Mar 22, 2023 at 05:05:10PM -0700, Peter Geoghegan wrote:
> > I'd go further than that myself: I haven't had any use for FPIs at
> > all. If I was going to do something with FPIs then I'd just use
> > pg_waldump, since I'd likely
On Wed, Mar 22, 2023 at 05:05:10PM -0700, Peter Geoghegan wrote:
> I'd go further than that myself: I haven't had any use for FPIs at
> all. If I was going to do something with FPIs then I'd just use
> pg_waldump, since I'd likely want to get them onto the filesystem for
> analysis anyway. (Just
On Tue, Mar 21, 2023 at 11:33 PM Michael Paquier wrote:
> > The new pg_get_wal_block_info outputs columns in an order that doesn't
> > seem like the most useful possible order to me. This gives us another
> > reason to have separate GetWALRecordInfo and GetWALBlockInfo utility
> > functions
On Wed, Mar 22, 2023 at 8:35 AM Melanie Plageman
wrote:
> After reading what you said, I was interested to see how substantial the
> I/O cost with non-compressed FPI would be.
>
> Using a patch with a parameter to pg_get_wal_block_info() to skip
> outputting FPI, I found that on a fast local nvme
On Mon, Mar 20, 2023 at 7:34 PM Peter Geoghegan wrote:
> On Sun, Mar 19, 2023 at 8:21 PM Kyotaro Horiguchi
> wrote:
> > It is not an issue with this patch, but as I look at this version, I'm
> > starting to feel uneasy about the subtle differences between what
> > GetWALRecordsInfo and
On Wed, Mar 22, 2023 at 11:35 AM Melanie Plageman
wrote:
>
> On Fri, Mar 17, 2023 at 8:51 PM Michael Paquier wrote:
> >
> > On Fri, Mar 17, 2023 at 04:36:58PM -0700, Peter Geoghegan wrote:
> > > I'm sure that they will do that much more than they would have
> > > otherwise. Since we'll have made
On Fri, Mar 17, 2023 at 8:51 PM Michael Paquier wrote:
>
> On Fri, Mar 17, 2023 at 04:36:58PM -0700, Peter Geoghegan wrote:
> > I'm sure that they will do that much more than they would have
> > otherwise. Since we'll have made pg_get_wal_block_info() so much more
> > useful than
On Mon, Mar 20, 2023 at 04:51:19PM -0700, Peter Geoghegan wrote:
> On Mon, Mar 20, 2023 at 4:34 PM Peter Geoghegan wrote:
>> I agree. A little redundancy is better when the alternative is fragile
>> code, and I'm pretty sure that that applies here -- there won't be
>> very many duplicated lines,
On Mon, Mar 20, 2023 at 05:00:25PM -0700, Peter Geoghegan wrote:
> I think that we should also make the description output column display
> NULLs for those records that don't output any description string. This
> at least includes the "FPI" record type from the "XLOG" rmgr.
> Alternatively, we
On Mon, Mar 20, 2023 at 4:51 PM Peter Geoghegan wrote:
> The new pg_get_wal_block_info outputs columns in an order that doesn't
> seem like the most useful possible order to me. This gives us another
> reason to have separate GetWALRecordInfo and GetWALBlockInfo utility
> functions rather than
On Mon, Mar 20, 2023 at 4:34 PM Peter Geoghegan wrote:
> I agree. A little redundancy is better when the alternative is fragile
> code, and I'm pretty sure that that applies here -- there won't be
> very many duplicated lines, and the final code will be significantly
> clearer. There can be a
On Sun, Mar 19, 2023 at 8:21 PM Kyotaro Horiguchi
wrote:
> The documentation has just one section titled "General Functions"
> which directly contains detailed explation of four functions, making
> it hard to get clear understanding of the available functions. I
> considered breaking it down
At Sat, 18 Mar 2023 10:08:53 +0530, Bharath Rupireddy
wrote in
> On Sat, Mar 18, 2023 at 1:06 AM Peter Geoghegan wrote:
> >
> > On Fri, Mar 17, 2023 at 12:20 AM Bharath Rupireddy
> > wrote:
> > > +1 for pg_get_wal_block_info emitting per-record WAL info too along
> > > with block info,
On Sat, Mar 18, 2023 at 1:06 AM Peter Geoghegan wrote:
>
> On Fri, Mar 17, 2023 at 12:20 AM Bharath Rupireddy
> wrote:
> > +1 for pg_get_wal_block_info emitting per-record WAL info too along
> > with block info, attached v2 patch does that. IMO, usability wins the
> > race here.
>
> I think that
On Fri, Mar 17, 2023 at 06:09:05PM -0700, Peter Geoghegan wrote:
>> This said, your point about having rec_blk_ref reported as an empty
>> string rather than NULL if there are no block references does not feel
>> natural to me, either.. Reporting NULL would be better.
>
> You have it backwards.
On Fri, Mar 17, 2023 at 5:51 PM Michael Paquier wrote:
> Yes. The CPU cost is one thing, but I am also worrying about the
> I/O cost with a tuplestore spilling to disk a large number of FPIs,
> and some workloads can generate WAL so as FPIs is what makes for most
> of the contents stored in the
On Fri, Mar 17, 2023 at 04:36:58PM -0700, Peter Geoghegan wrote:
> I'm sure that they will do that much more than they would have
> otherwise. Since we'll have made pg_get_wal_block_info() so much more
> useful than pg_get_wal_records_info() for many important use cases.
> Why is that a bad thing?
On Fri, Mar 17, 2023 at 4:11 PM Michael Paquier wrote:
> FWIW, I am not sure that it is a good idea and that we'd better not
> encourage too much the use of block_info() across a large range of
> WAL, which is what this function will make users eager to do in this
> case as it is possible to
On Fri, Mar 17, 2023 at 12:36:04PM -0700, Peter Geoghegan wrote:
> I think that this direction makes a lot of sense. Under this scheme,
> we still have pg_get_wal_records_info(), which is more or less an SQL
> interface to the information that pg_waldump presents by default.
> That's important
On Fri, Mar 17, 2023 at 12:20 AM Bharath Rupireddy
wrote:
> +1 for pg_get_wal_block_info emitting per-record WAL info too along
> with block info, attached v2 patch does that. IMO, usability wins the
> race here.
I think that this direction makes a lot of sense. Under this scheme,
we still have
On Fri, Mar 17, 2023 at 7:33 AM Peter Geoghegan wrote:
>
> > IIUC, the concern raised so far in this thread is not just on the
> > performance of JOIN queries to get both block info and record level
> > info, but on ease of using pg_walinspect functions. If
> > pg_get_wal_block_info emits the
On Thu, Mar 16, 2023 at 2:19 AM Bharath Rupireddy
wrote:
> On Wed, Mar 15, 2023 at 12:20 PM Michael Paquier wrote:
> > I am not sure to get the concern here. As long as one is smart enough
> > with SQL, there is no need to perform a double scan of the contents of
> > pg_wal with a large scan on
On Wed, Mar 15, 2023 at 12:20 PM Michael Paquier wrote:
>
> On Tue, Mar 14, 2023 at 06:50:15PM -0700, Peter Geoghegan wrote:
> > On Tue, Mar 14, 2023 at 5:34 PM Melanie Plageman
> > wrote:
> >> Well, I think if you only care about the WAL record-level information
> >> and not the block-level
On Wed, Mar 15, 2023 at 12:13:56PM +0530, Bharath Rupireddy wrote:
> How about something like the attached? It adds the per-record columns
> to pg_get_wal_block_info() avoiding "possibly expensive" joins with
> pg_get_wal_records_info().
>
> With this, pg_get_wal_records_info() too will be useful
On Tue, Mar 14, 2023 at 06:50:15PM -0700, Peter Geoghegan wrote:
> On Tue, Mar 14, 2023 at 5:34 PM Melanie Plageman
> wrote:
>> Well, I think if you only care about the WAL record-level information
>> and not the block-level information, having the WAL record information
>> denormalized like that
On Wed, Mar 15, 2023 at 7:20 AM Peter Geoghegan wrote:
>
> > But, perhaps you are suggesting a parameter to pg_get_wal_records_info()
> > like "with_block_info" or something, which produces the full
> > denormalized block + record output?
>
> I was thinking of something like that, yes -- though
On Tue, Mar 14, 2023 at 5:34 PM Melanie Plageman
wrote:
> On Tue, Mar 14, 2023 at 6:57 PM Peter Geoghegan wrote:
> > Why doesn't it already work like this? Why do we need a separate
> > pg_get_wal_block_info() function at all?
>
> Well, I think if you only care about the WAL record-level
On Tue, Mar 14, 2023 at 6:57 PM Peter Geoghegan wrote:
>
> On Tue, Mar 14, 2023 at 3:34 PM Melanie Plageman
> wrote:
> > After patching master to add in the columns from
> > pg_get_wal_records_info() which are not returned by
> > pg_get_wal_block_info() (except block_ref column of course), this
On Tue, Mar 14, 2023 at 3:34 PM Melanie Plageman
wrote:
> After patching master to add in the columns from
> pg_get_wal_records_info() which are not returned by
> pg_get_wal_block_info() (except block_ref column of course), this query:
>
> SELECT COUNT(*) FROM pg_get_wal_block_info(:start_lsn,
I'm excited to see that pg_get_wal_block_info() was merged. Thanks for
working on this!
Apologies for jumping back in here a bit late. I've been playing around
with it and wanted to comment on the performance of JOINing to
pg_get_wal_records_info().
On Tue, Mar 7, 2023 at 7:08 AM Matthias van de
On Fri, Mar 10, 2023 at 06:50:05AM +0530, Bharath Rupireddy wrote:
> Perhaps what is proposed here
> https://www.postgresql.org/message-id/calj2acwqj+m0hoqj9qkav2uqfq97yk5jn2modfkcxusxsyp...@mail.gmail.com
> might help and avoid many errors around input LSN validations. Let's
> discuss that in
On Fri, Mar 10, 2023 at 6:43 AM Michael Paquier wrote:
>
> I'd really like to do something about the errors we raise in the
> module when specifying LSNs in the future for this release, now. I
> got annoyed by it again this morning while doing \watch queries that
> kept failing randomly while
On Thu, Mar 09, 2023 at 03:37:21PM +0900, Michael Paquier wrote:
> Okay, thanks. Let's use that, then.
I have done one pass over that today, and applied it. Thanks!
I'd really like to do something about the errors we raise in the
module when specifying LSNs in the future for this release, now.
On Thu, Mar 09, 2023 at 09:52:57AM +0530, Bharath Rupireddy wrote:
> Hm, then, +1 for v3.
Okay, thanks. Let's use that, then.
--
Michael
signature.asc
Description: PGP signature
On Thu, Mar 9, 2023 at 7:34 AM Kyotaro Horiguchi
wrote:
>
> > In short, my choice would still be simplicity here with v3, I guess.
>
> FWIW, I slightly prefer v3 for the reason I mentioned above.
Hm, then, +1 for v3.
FWIW, I quickly tried to hit that case where a single WAL record has
On Thu, Mar 09, 2023 at 09:46:12AM +0900, Kyotaro Horiguchi wrote:
> Although I'm not strongly opposed to pfreeing them, I'm not sure I
> like the way the patch frees them. The life times of all of raw_data,
> raw_page and flags are within a block. They can be freed
> unconditionally after they
At Wed, 8 Mar 2023 20:18:06 +0530, Bharath Rupireddy
wrote in
> On Wed, Mar 8, 2023 at 4:23 PM Michael Paquier wrote:
> >
> > On Wed, Mar 08, 2023 at 04:01:56PM +0530, Bharath Rupireddy wrote:
> > > I understand that performance is critical here but we need to ensure
> > > memory is used
On Wed, Mar 8, 2023 at 4:23 PM Michael Paquier wrote:
>
> On Wed, Mar 08, 2023 at 04:01:56PM +0530, Bharath Rupireddy wrote:
> > I understand that performance is critical here but we need to ensure
> > memory is used wisely. Therefore, I'd still vote to free at least the
> > major contributors
On Wed, Mar 08, 2023 at 04:01:56PM +0530, Bharath Rupireddy wrote:
> I understand that performance is critical here but we need to ensure
> memory is used wisely. Therefore, I'd still vote to free at least the
> major contributors here, that is, pfree(raw_data);, pfree(raw_page);
> and
On Wed, Mar 8, 2023 at 12:58 PM Michael Paquier wrote:
>
> On Tue, Mar 07, 2023 at 03:56:22PM +0530, Bharath Rupireddy wrote:
> > 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
On Tue, Mar 07, 2023 at 03:56:22PM +0530, Bharath Rupireddy wrote:
> 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
On Tue, 7 Mar 2023 at 01:34, Michael Paquier wrote:
>
> On Mon, Mar 06, 2023 at 04:08:28PM +0100, Matthias van de Meent wrote:
> > On Mon, 6 Mar 2023 at 15:40, Bharath Rupireddy
> >> IMO, pg_get_wal_records_extended_info as proposed doesn't look good to
> >> me as it outputs most of the columns
On Tue, Mar 7, 2023 at 12:48 PM Michael Paquier 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
At Tue, 7 Mar 2023 16:18:21 +0900, Michael Paquier wrote
in
> 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
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[]
At Tue, 7 Mar 2023 14:44:49 +0900, Michael Paquier wrote
in
> On Tue, Mar 07, 2023 at 11:17:45AM +0900, Kyotaro Horiguchi wrote:
> > Thus I'm inclined to agree with Michael's suggestion of creating a new
> > normalized set-returning function that returns information of
> > "blocks".
>
> Just
On Tue, Mar 07, 2023 at 11:17:45AM +0900, Kyotaro Horiguchi wrote:
> Thus I'm inclined to agree with Michael's suggestion of creating a new
> normalized set-returning function that returns information of
> "blocks".
Just to be clear here, I am not suggesting to add a new function for
only the
At Tue, 7 Mar 2023 09:34:24 +0900, Michael Paquier wrote
in
> On Mon, Mar 06, 2023 at 04:08:28PM +0100, Matthias van de Meent wrote:
> > On Mon, 6 Mar 2023 at 15:40, Bharath Rupireddy
> >> IMO, pg_get_wal_records_extended_info as proposed doesn't look good to
> >> me as it outputs most of the
On Mon, Mar 06, 2023 at 04:08:28PM +0100, Matthias van de Meent wrote:
> On Mon, 6 Mar 2023 at 15:40, Bharath Rupireddy
>> IMO, pg_get_wal_records_extended_info as proposed doesn't look good to
>> me as it outputs most of the columns that are already given by
>> pg_get_wal_records_info.What I
On Mon, 6 Mar 2023 at 15:40, Bharath Rupireddy
wrote:
>
> On Thu, Mar 2, 2023 at 9:47 PM Melanie Plageman
> wrote:
> >
> > > However, I am mainly looking for feedback about whether or not others
> > > would find this useful, and, if so, what columns they would like to see
> > > in the returned
On Thu, Mar 2, 2023 at 9:47 PM Melanie Plageman
wrote:
>
> > However, I am mainly looking for feedback about whether or not others
> > would find this useful, and, if so, what columns they would like to see
> > in the returned tuplestore.
IMO, pg_get_wal_records_extended_info as proposed doesn't
On Thu, Mar 02, 2023 at 11:17:05AM -0500, Melanie Plageman wrote:
> Thinking about this more, it could make sense to have a function which
> gives you this extended block information and has a parameter like
> with_fpi which would include the information returned by
> pg_get_wal_fpi_info(). It
On Wed, Mar 1, 2023 at 12:51 PM Melanie Plageman
wrote:
> When using pg_walinspect, and calling functions like
> pg_get_wal_records_info(), I often wish that the various information in
> the block_ref column was separated out into columns so that I could
> easily access them and pass them to
Hi,
When using pg_walinspect, and calling functions like
pg_get_wal_records_info(), I often wish that the various information in
the block_ref column was separated out into columns so that I could
easily access them and pass them to various other functions to add
information -- like getting the
75 matches
Mail list logo
