Re: Patch for fail-back without fresh backup

On Tue, Oct 8, 2013 at 6:37 PM, Pavan Deolasee <pavan.deolasee@gmail.com> wrote:
>
> On Tue, Oct 8, 2013 at 2:33 PM, Sawada Masahiko <sawada.mshk@gmail.com>
> wrote:
>>
>> On Fri, Oct 4, 2013 at 4:32 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>> >
>> I attached the v12 patch which have modified based on above suggestions.
>
>
> There are still some parts of this design/patch which I am concerned about.
>
> 1. The design clubs synchronous standby and failback safe standby rather
> very tightly. IIRC this is based on the feedback you received early, so my
> apologies for raising it again so late.
>  a. GUC synchrnous_standby_names is used to name synchronous as well as
> failback safe standbys. I don't know if that will confuse users.

With currently the patch, user can specify the failback safe standby
and sync replication standby at same server.
synchronous_standby_names
So I was thinking that it will not confuse users.

>  b. synchronous_commit's value will also control whether a sync/async
> failback safe standby wait for remote write or flush. Is that reasonable ?
> Or should there be a different way to configure the failback safe standby's
> WAL safety ?

synchronous_commit's values can not control whether sync sync/async
failback safe standby wait level.
On data page flush, failback safe standby waits for only flush.
Should we also allow to wait for remote write?

>
> 2. With the current design/implementation, user can't configure a
> synchronous and an async failback safe standby at the same time. I think we
> discussed this earlier and there was an agreement on the limitation. Just
> wanted to get that confirmed again.
>

yes, user can't configure sync standby and async failback safe standby
at the same time.
The currently patch supports following cases.
- sync standby and make same as failback safe standby
- async standby and make same as failback safe standby


> 3. SyncRepReleaseWaiters() does not know whether its waking up backends
> waiting for sync rep or failback safe rep. Is that ok ? For example, I found
> that the elog() message announcing next takeover emitted by the function may
> look bad. Since changing synchronous_transfer requires server restart, we
> can teach SyncRepReleaseWaiters() to look at that parameter to figure out
> whether the standby is sync and/or failback safe standby.

I agree with you.
Are you saying about following comment?
if (announce_next_takeover)
{   announce_next_takeover = false;   ereport(LOG,           (errmsg("standby \"%s\" is now the synchronous standby
with priority %u",                   application_name,
MyWalSnd->sync_standby_priority)));
}

>
> 4. The documentation still need more work to clearly explain the use case.

Understood. we will more work to clearly explain the use case.

>
> 5. Have we done any sort of stress testing of the patch ? If there is a bug,
> the data corruption at the master can go unnoticed. So IMHO we need many
> crash recovery tests to ensure that the patch is functionally correct.
>

I have done several testing of the patch. And I have confirmed that
data page is not flushed to disk
when the master server has not been receive the reply from the standby
server. I used pg_filedump.
To ensure that the patch is functionally correct, what test should we do?

Regards,

-------
Sawada Masahiko