On Mon, Dec 26, 2022 at 4:18 PM Bharath Rupireddy
wrote:
>
> On Mon, Dec 26, 2022 at 12:59 PM Michael Paquier wrote:
>
> +1. I think this feature will also be useful in pg_walinspect.
Just for the record - here's the pg_walinspect function to extract
FPIs from WAL records -
https://www.postgresq
On Mon, Dec 26, 2022 at 04:18:18PM +0530, Bharath Rupireddy wrote:
> +1. I think this feature will also be useful in pg_walinspect.
> However, I'm a bit concerned that it can flood the running database
> disk - if someone generates a lot of FPI files.
pg_read_file() and pg_waldump can be fed absol
On Mon, Dec 26, 2022 at 02:39:03PM -0600, Justin Pryzby wrote:
> fclose() should be tested, too:
Sure. Done that too, and applied the change after a last lookup.
--
Michael
signature.asc
Description: PGP signature
On Mon, Dec 26, 2022 at 04:28:52PM +0900, Michael Paquier wrote:
> Comments?
> + file = fopen(filename, PG_BINARY_W);
> + if (!file)
> + pg_fatal("could not open file \"%s\": %m", filename);
> +
> + if (fwrite(page, BLCKSZ, 1, file) != 1)
> +
> On Dec 26, 2022, at 1:29 AM, Michael Paquier wrote:
>
> On Sat, Dec 24, 2022 at 06:23:29PM +0530, Bharath Rupireddy wrote:
>> Thanks for the patch. I've made the above change as well as renamed
>> the test file name to be save_fpi.pl, everything else remains the same
>> as v11. Here's the v12
On Mon, Dec 26, 2022 at 12:59 PM Michael Paquier wrote:
>
> I have done a review of that, and here are my notes:
> - The variable names were a bit inconsistent, so I have switched most
> of the new code to use "fullpage".
>
> - The new test has been renamed.
>
> - RestoreBlockImage() would report
On Sat, Dec 24, 2022 at 06:23:29PM +0530, Bharath Rupireddy wrote:
> Thanks for the patch. I've made the above change as well as renamed
> the test file name to be save_fpi.pl, everything else remains the same
> as v11. Here's the v12 patch which LGTM. I'll mark it as RfC -
> https://commitfest.pos
On Sat, Dec 24, 2022 at 12:58 AM David Christensen
wrote:
>
> On Fri, Dec 23, 2022 at 12:57 PM David Christensen
> wrote:
> >
> > On Wed, Dec 21, 2022 at 5:47 AM Bharath Rupireddy
> > wrote:
>
> > > 2. +$node->init(extra => ['-k'], allows_streaming => 1);
> > > When enabled with allows_streaming
On Fri, Dec 23, 2022 at 12:57 PM David Christensen
wrote:
>
> On Wed, Dec 21, 2022 at 5:47 AM Bharath Rupireddy
> wrote:
[snip]
> > 2. +$node->init(extra => ['-k'], allows_streaming => 1);
> > When enabled with allows_streaming, there are a bunch of things that
> > happen to the node while init
On Mon, Dec 19, 2022 at 12:23 AM Michael Paquier wrote:
>
> On Thu, Dec 15, 2022 at 05:17:46PM -0600, David Christensen wrote:
> > On Thu, Dec 15, 2022 at 12:36 AM Michael Paquier
> > wrote:
> > This v10 should incorporate your feedback as well as Bharath's.
>
> Thanks for the new version. I ha
On Wed, Dec 21, 2022 at 5:47 AM Bharath Rupireddy
wrote:
>
> On Fri, Dec 16, 2022 at 4:47 AM David Christensen
> wrote:
> >
> > On Thu, Dec 15, 2022 at 12:36 AM Michael Paquier
> > wrote:
> > >
> > > On Wed, Dec 14, 2022 at 04:44:34PM -0600, David Christensen wrote:
> > > > I can get one sent i
On Fri, Dec 16, 2022 at 4:47 AM David Christensen
wrote:
>
> On Thu, Dec 15, 2022 at 12:36 AM Michael Paquier wrote:
> >
> > On Wed, Dec 14, 2022 at 04:44:34PM -0600, David Christensen wrote:
> > > I can get one sent in tomorrow.
>
> This v10 should incorporate your feedback as well as Bharath's.
On Thu, Dec 15, 2022 at 05:17:46PM -0600, David Christensen wrote:
> On Thu, Dec 15, 2022 at 12:36 AM Michael Paquier wrote:
> This v10 should incorporate your feedback as well as Bharath's.
Thanks for the new version. I have minor comments.
>> It seems to me that you could allow things to work
On Thu, Dec 15, 2022 at 12:36 AM Michael Paquier wrote:
>
> On Wed, Dec 14, 2022 at 04:44:34PM -0600, David Christensen wrote:
> > I can get one sent in tomorrow.
This v10 should incorporate your feedback as well as Bharath's.
> -XLogRecordHasFPW(XLogReaderState *record)
> +XLogRecordHasFPI(XLog
On Wed, Dec 14, 2022 at 04:44:34PM -0600, David Christensen wrote:
> I can get one sent in tomorrow.
-XLogRecordHasFPW(XLogReaderState *record)
+XLogRecordHasFPI(XLogReaderState *record)
This still refers to a FPW, so let's leave that out as well as any
renamings of this kind..
+ if (config.sav
Hi Bharath,
I can get one sent in tomorrow.
Thanks,
David
On Thu, Nov 17, 2022 at 10:02 PM David Christensen
wrote:
>
> On Wed, Nov 16, 2022 at 3:30 AM Bharath Rupireddy
> wrote:
> > 1.
> > -if (config.filter_by_fpw && !XLogRecordHasFPW(xlogreader_state))
> > +if (config.filter_by_fpw && !XLogRecordHasFPI(xlogreader_state))
> > These cha
On Wed, Nov 16, 2022 at 3:30 AM Bharath Rupireddy
wrote:
> 1.
> -if (config.filter_by_fpw && !XLogRecordHasFPW(xlogreader_state))
> +if (config.filter_by_fpw && !XLogRecordHasFPI(xlogreader_state))
> These changes are not related to this feature, hence renaming those
> variables/fu
On Wed, Nov 16, 2022 at 1:20 AM David Christensen
wrote:
>
> Enclosed is v9.
>
> - code style consistency (FPI instead of FPW) internally.
> - cleanup of no-longer needed checksum-related pieces from code and tests.
> - test cleanup/simplification.
> - other comment cleanup.
>
> Passes all CI chec
Enclosed is v9.
- code style consistency (FPI instead of FPW) internally.
- cleanup of no-longer needed checksum-related pieces from code and tests.
- test cleanup/simplification.
- other comment cleanup.
Passes all CI checks.
Best,
David
v9-0001-Teach-pg_waldump-to-extract-FPIs-from-the-WAL-
On Tue, Nov 15, 2022 at 4:41 AM Bharath Rupireddy
wrote:
>
> On Tue, Nov 15, 2022 at 1:29 AM David Christensen
> wrote:
> >
> > Enclosed is v8, which uses the replication slot method to retain WAL
> > as well as fsync'ing the output directory when everything is done.
>
> Thanks. It mostly is in g
On Tue, Nov 15, 2022 at 1:29 AM David Christensen
wrote:
>
> Enclosed is v8, which uses the replication slot method to retain WAL
> as well as fsync'ing the output directory when everything is done.
Thanks. It mostly is in good shape. However, few more comments:
1.
+if it does not exist.
Enclosed is v8, which uses the replication slot method to retain WAL
as well as fsync'ing the output directory when everything is done.
v8-0001-Teach-pg_waldump-to-extract-FPIs-from-the-WAL-str.patch
Description: Binary data
On Fri, Nov 11, 2022 at 4:57 AM Bharath Rupireddy
wrote:
> > Well if it's not the same output then I guess you're right and there's
> > not a use for the `--fixup` mode. By the same token, I'd say
> > calculating/setting the checksum also wouldn't need to be done, we
> > should just include the p
On Fri, Nov 11, 2022 at 8:15 AM Justin Pryzby wrote:
>
> On Wed, Nov 09, 2022 at 02:37:29PM -0600, David Christensen wrote:
> > On Wed, Nov 9, 2022 at 2:08 PM David Christensen
> > wrote:
> > > Justin sez:
> > > > I was wondering if there's any reason to do "CREATE DATABASE". The vast
> > > > m
On Wed, Nov 09, 2022 at 02:37:29PM -0600, David Christensen wrote:
> On Wed, Nov 9, 2022 at 2:08 PM David Christensen
> wrote:
> > Justin sez:
> > > I was wondering if there's any reason to do "CREATE DATABASE". The vast
> > > majority of TAP tests don't.
> > >
> > > $ git grep -ho 'safe_psql[^
On Thu, Nov 10, 2022 at 9:52 PM David Christensen
wrote:
>
> > > > 2. I'm unable to understand the use-case for --fixup-fpi option.
> > > > pg_waldump is supposed to be just WAL reader, and must not return any
> > > > modified information, with --fixup-fpi option, the patch violates this
> > > > p
On Thu, Nov 10, 2022 at 2:33 AM Bharath Rupireddy
wrote:
>
> On Thu, Nov 10, 2022 at 1:31 AM David Christensen
> wrote:
> >
>
> Thanks for providing the v7 patch, please see my comments and responses below.
Hi Bharath, Thanks for the feedback.
> > > 2. I'm unable to understand the use-case for
On Thu, Nov 10, 2022 at 1:31 AM David Christensen
wrote:
>
Thanks for providing the v7 patch, please see my comments and responses below.
> > 2. I'm unable to understand the use-case for --fixup-fpi option.
> > pg_waldump is supposed to be just WAL reader, and must not return any
> > modified in
On Wed, Nov 9, 2022 at 2:08 PM David Christensen
wrote:
> Justin sez:
> > I was wondering if there's any reason to do "CREATE DATABASE". The vast
> > majority of TAP tests don't.
> >
> > $ git grep -ho 'safe_psql[^ ]*' '*pl' |sort |uniq -c |sort -nr |head
> >1435 safe_psql('postgres',
> >
> > 6.
> > +if (dir_status == 0 && mkdir(config.save_fpw_path, 0700) < 0)
> > Use pg_dir_create_mode instead of hard-coded 0007?
>
> I think I thought of that when I first looked at the patch ... but, I'm
> not sure, since it says:
>
> src/include/common/file_perm.h-/* Modes for creating d
On Wed, Nov 9, 2022 at 6:30 AM Bharath Rupireddy
wrote:
>
> On Wed, Nov 9, 2022 at 5:08 AM David Christensen
> wrote:
> >
> > Enclosed is v6, which squashes your refactor and adds the additional
> > recent suggestions.
>
> Thanks for working on this feature. Here are some comments for now. I
> ha
On 2022-Nov-09, Justin Pryzby wrote:
> On Wed, Nov 09, 2022 at 06:00:40PM +0530, Bharath Rupireddy wrote:
> > 1. For ease of review, please split the test patch to 0002.
>
> This is just my opinion, but .. why ? Since it's easy to
> filter/skip/display a file, I don't think it's usually useful
On Wed, Nov 09, 2022 at 06:00:40PM +0530, Bharath Rupireddy wrote:
> On Wed, Nov 9, 2022 at 5:08 AM David Christensen
> wrote:
> >
> > Enclosed is v6, which squashes your refactor and adds the additional
> > recent suggestions.
>
> Thanks for working on this feature. Here are some comments for n
On Wed, Nov 9, 2022 at 5:08 AM David Christensen
wrote:
>
> Enclosed is v6, which squashes your refactor and adds the additional
> recent suggestions.
Thanks for working on this feature. Here are some comments for now. I
haven't looked at the tests, I will take another look at the code and
tests
The following review has been posted through the commitfest application:
make installcheck-world: tested, failed
Implements feature: tested, failed
Spec compliant: tested, failed
Documentation:tested, failed
Hello,
I tested this patch on Linux and there is no problem.
Enclosed is v6, which squashes your refactor and adds the additional
recent suggestions.
Thanks!
v6-0001-Teach-pg_waldump-to-extract-FPIs-from-the-WAL-str.patch
Description: Binary data
On Tue, Nov 8, 2022 at 4:45 PM Justin Pryzby wrote:
>
> On Mon, Nov 07, 2022 at 05:01:01PM -0600, David Christensen wrote:
> > Hi Justin et al,
> >
> > Enclosed is v5 of this patch which now passes the CirrusCI checks for
> > all supported OSes. I went ahead and reworked the test a bit so it's a
>
On Mon, Nov 07, 2022 at 05:01:01PM -0600, David Christensen wrote:
> Hi Justin et al,
>
> Enclosed is v5 of this patch which now passes the CirrusCI checks for
> all supported OSes. I went ahead and reworked the test a bit so it's a
> little more amenable to the OS-agnostic approach for testing.
Hi Justin et al,
Enclosed is v5 of this patch which now passes the CirrusCI checks for
all supported OSes. I went ahead and reworked the test a bit so it's a
little more amenable to the OS-agnostic approach for testing.
Best,
David
v5-0001-Teach-pg_waldump-to-extract-FPIs-from-the-WAL-str.patc
On Fri, Nov 4, 2022 at 1:38 PM Justin Pryzby wrote:
> > > As I recall, that's due to relying on "cp". And "rsync", which
> > > shouldn't be assumed to exist by regression tests).
Will poke around other TAP tests to see if there's a more consistent
interface, what perl version we can assume and a
On Fri, Nov 04, 2022 at 09:16:29AM -0500, David Christensen wrote:
> On Nov 4, 2022, at 9:02 AM, Justin Pryzby wrote:
> > On Fri, Nov 04, 2022 at 11:52:59AM +0900, Ian Lawrence Barwick wrote:
> >> 2022年5月3日(火) 8:45 David Christensen :
> >>>
> >>> ...and pushing a couple fixups pointed out by cfbo
On Nov 4, 2022, at 9:02 AM, Justin Pryzby wrote:
>
> On Fri, Nov 04, 2022 at 11:52:59AM +0900, Ian Lawrence Barwick wrote:
>> 2022年5月3日(火) 8:45 David Christensen :
>>>
>>> ...and pushing a couple fixups pointed out by cfbot, so here's v4.
>>
>> cfbot reports the patch no longer applies [1]. As
On Fri, Nov 04, 2022 at 11:52:59AM +0900, Ian Lawrence Barwick wrote:
> 2022年5月3日(火) 8:45 David Christensen :
> >
> > ...and pushing a couple fixups pointed out by cfbot, so here's v4.
>
> cfbot reports the patch no longer applies [1]. As CommitFest 2022-11 is
> currently underway, this would be a
2022年5月3日(火) 8:45 David Christensen :
>
> ...and pushing a couple fixups pointed out by cfbot, so here's v4.
Hi
cfbot reports the patch no longer applies [1]. As CommitFest 2022-11 is
currently underway, this would be an excellent time to update the patch.
[1] http://cfbot.cputube.org/patch_40_3
On Tue, May 03, 2022 at 10:34:41AM +0200, Alvaro Herrera wrote:
> I remember Greg Mullane posted a tool that attempts to correct page CRC
> mismatches[1]. This new tool might be useful to feed healing attempts,
> too. (It's of course not in any way a *solution*, because the page
> might have been
On 2022-Apr-27, Michael Paquier wrote:
> On Tue, Apr 26, 2022 at 01:15:05PM -0500, David Christensen wrote:
> > True. :-) This does seem like a tool geared towards "expert mode", so
> > maybe we just assume if you need it you know what you're doing?
>
> This is definitely an expert mode toy.
I r
...and pushing a couple fixups pointed out by cfbot, so here's v4.
On Mon, May 2, 2022 at 8:42 AM David Christensen
wrote:
>
> Enclosed is v3 of this patch; this adds two modes for this feature,
> one with the raw page `--save-fullpage/-W` and one with the
> LSN+checksum fixups `--save-fullpage-
Enclosed is v3 of this patch; this adds two modes for this feature,
one with the raw page `--save-fullpage/-W` and one with the
LSN+checksum fixups `--save-fullpage-fixup/-X`.
I've added at least some basic sanity-checking of the underlying
feature, as well as run the test file and the changes to
On Tue, Apr 26, 2022 at 01:15:05PM -0500, David Christensen wrote:
> True. :-) This does seem like a tool geared towards "expert mode", so
> maybe we just assume if you need it you know what you're doing?
This is definitely an expert mode toy.
--
Michael
signature.asc
Description: PGP signature
On Mon, Apr 25, 2022 at 9:42 PM Michael Paquier wrote:
> On Mon, Apr 25, 2022 at 10:24:52AM -0500, David Christensen wrote:
> > On Mon, Apr 25, 2022 at 6:03 AM Bharath Rupireddy
> > wrote:
> >> Thanks for working on this. I'm just thinking if we can use these FPIs
> >> to repair the corrupted pag
On Mon, Apr 25, 2022 at 9:54 PM Michael Paquier wrote:
> On Mon, Apr 25, 2022 at 10:11:10AM -0500, David Christensen wrote:
> > On Mon, Apr 25, 2022 at 1:11 AM Michael Paquier wrote:
> >> I don't think that there is any need to rely on a new logic if there
> >> is already some code in place able
On Mon, Apr 25, 2022 at 10:11:10AM -0500, David Christensen wrote:
> On Mon, Apr 25, 2022 at 1:11 AM Michael Paquier wrote:
>> I don't think that there is any need to rely on a new logic if there
>> is already some code in place able to do the same work. See
>> verify_dir_is_empty_or_create() in
On Mon, Apr 25, 2022 at 10:24:52AM -0500, David Christensen wrote:
> On Mon, Apr 25, 2022 at 6:03 AM Bharath Rupireddy
> wrote:
>> Thanks for working on this. I'm just thinking if we can use these FPIs
>> to repair the corrupted pages? I would like to understand more
>> detailed usages of the FPIs
On Mon, Apr 25, 2022 at 6:03 AM Bharath Rupireddy
wrote:
> Thanks for working on this. I'm just thinking if we can use these FPIs
> to repair the corrupted pages? I would like to understand more
> detailed usages of the FPIs other than inspecting with pageinspect.
My main use case was for being a
On Mon, Apr 25, 2022 at 2:00 AM Drouvot, Bertrand wrote:
>
> Hi,
>
> On 4/25/22 8:11 AM, Michael Paquier wrote:
> > On Sat, Apr 23, 2022 at 01:43:36PM -0500, David Christensen wrote:
> >> Hi Matthias, great point. Enclosed is a revised version of the patch
> >> that adds the fork identifier to th
On Mon, Apr 25, 2022 at 1:11 AM Michael Paquier wrote:
>
> On Sat, Apr 23, 2022 at 01:43:36PM -0500, David Christensen wrote:
> > Hi Matthias, great point. Enclosed is a revised version of the patch
> > that adds the fork identifier to the end if it's a non-main fork.
>
> Like Alvaro, I have seen
On Sat, Apr 23, 2022 at 4:21 AM David Christensen
wrote:
>
> Hi -hackers,
>
> Enclosed is a patch to allow extraction/saving of FPI from the WAL
> stream via pg_waldump.
>
> Description from the commit:
>
> Extracts full-page images from the WAL stream into a target directory,
> which must be empt
On Sat, Apr 23, 2022 at 01:43:36PM -0500, David Christensen wrote:
> Hi Matthias, great point. Enclosed is a revised version of the patch
> that adds the fork identifier to the end if it's a non-main fork.
Like Alvaro, I have seen cases where this would have been really
handy. So +1 from me, as
On Sat, Apr 23, 2022 at 9:49 AM Matthias van de Meent
wrote:
> Regardless of my (lack of) opinion on the inclusion of this patch in
> PG (I did not significantly review this patch); I noticed that you do
> not yet identify the 'fork' of the FPI in the file name.
>
> A lack of fork identifier in t
On Sat, 23 Apr 2022 at 00:51, David Christensen
wrote:
>
> Hi -hackers,
>
> Enclosed is a patch to allow extraction/saving of FPI from the WAL
> stream via pg_waldump.
>
> Description from the commit:
>
> Extracts full-page images from the WAL stream into a target directory,
> which must be empty
On 2022-Apr-22, David Christensen wrote:
> Hi -hackers,
>
> Enclosed is a patch to allow extraction/saving of FPI from the WAL
> stream via pg_waldump.
I already wrote and posted a patch to do exactly this, and found it the
only way to fix a customer problem, so +1 on having this feature. I
hav
Hi -hackers,
Enclosed is a patch to allow extraction/saving of FPI from the WAL
stream via pg_waldump.
Description from the commit:
Extracts full-page images from the WAL stream into a target directory,
which must be empty or not
exist. These images are subject to the same filtering rules as no
63 matches
Mail list logo
