Re: pg_rewind with cascade standby doesn't work well

From: Kuwamura Masaki <kuwamura(at)db(dot)is(dot)i(dot)nagoya-u(dot)ac(dot)jp>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Aleksander Alekseev <aleksander(at)timescale(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Subject: Re: pg_rewind with cascade standby doesn't work well
Date: 2023-10-06 07:53:20
Message-ID: CAMyC8qoF+WpyHD2cYtE-Xjb53cfe_6oJoTZ32X5CG3n94c-rCQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for your review!

2023年9月27日(水) 8:33 Michael Paquier <michael(at)paquier(dot)xyz>:

> On Tue, Sep 26, 2023 at 06:44:50PM +0300, Aleksander Alekseev wrote:
> >> And also, I'm afraid that I'm not sure what kind of tests I have to make
> >> for fix this behavior. Would you mind giving me some advice?
> >
> > Personally I would prefer not to increase the scope of work. Your TAP
> > test added in 0001 seems to be adequate.
>
> Yeah, agreed. I'm OK with what you are proposing, basically (the
> test could be made a bit cheaper actually).

I guess you meant that it contains an unnecessary insert and wait.
I fixed this and some incorrect comments caused by copy & paste.
Please see the attached patch.

>
/*
> - * For Hot Standby, we could treat this like a Shutdown
> Checkpoint,
> - * but this case is rarer and harder to test, so the benefit
> doesn't
> - * outweigh the potential extra cost of maintenance.
> + * For Hot Standby, we could treat this like an end-of-recovery
> checkpoint
> */
> + RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_WAIT);
>
> I don't understand what you want to change here. Archive recovery and
> crash recovery are two different things, still this code would be
> triggered even if there is no standby.signal, aka the node is not a
> standby. Why isn't this stuff conditional?

You are absolutely right. It should only run in standby mode.
Also, according to the document[1], a server can be "Hot Standby" even if
it is
not in standby mode (i.e. when it is in archive recovery mode).
So I fixed the comment above `RequestCheckpoint()`.

[1]: https://www.postgresql.org/docs/current/hot-standby.html

I hope you will review it again.

Regards,

Masaki Kuwamura

Attachment Content-Type Size
v3-0001-pg_rewind-Fix-bug-using-cascade-standby-as-source.patch application/octet-stream 4.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-10-06 07:58:37 Re: Pre-proposal: unicode normalized text
Previous Message Maxim Orlov 2023-10-06 07:50:11 Re: should frontend tools use syncfs() ?