Re: [HACKERS] Replication status in logical replication

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Tels <nospam-pg-abuse(at)bloodgate(dot)com>
Cc: Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Vaishnavi Prabakaran <vaishnaviprabakaran(at)gmail(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Replication status in logical replication
Date: 2017-12-26 10:26:27
Message-ID: CAD21AoByhYcQJDRqX9GXRq_zCfPOLa=_mJbUe7iA8xrLDAc2gQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Tue, Dec 26, 2017 at 6:19 PM, Tels <nospam-pg-abuse(at)bloodgate(dot)com> wrote:
> Moin,
>
> On Mon, December 25, 2017 7:26 pm, Masahiko Sawada wrote:
>> On Tue, Dec 26, 2017 at 1:10 AM, Petr Jelinek
>> <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>>> On 21/11/17 22:06, Masahiko Sawada wrote:
>>>>
>>>> After investigation, I found out that my previous patch was wrong
>>>> direction. I should have changed XLogSendLogical() so that we can
>>>> check the read LSN and set WalSndCaughtUp = true even after read a
>>>> record without wait. Attached updated patch passed 'make check-world'.
>>>> Please review it.
>>>>
>>>
>>> Hi,
>>>
>>> This version looks good to me and seems to be in line with what we do in
>>> physical replication.
>>>
>>> Marking as ready for committer.
>
> (Sorry Masahiko, you'll get this twice, as fumbled the reply button.)
>
> I have not verifed that comment and/or code are correct, just a grammar fix:
>
> + /*
> + * If we've sent a record is at or beyond the flushed
> point, then
> + * we're caught up.
>
> That should read more like this:
>
> "If we've sent a record that is at or beyond the flushed point, we have
> caught up."
>

Thank you for reviewing the patch!
Actually, that comment is inspired by the comment just below comment.
ISTM it's better to fix both if grammar of them is not appropriate.

+
+ /*
+ * If we've sent a record that is at or beyond the flushed point, we
+ * have caught up.
+ */
+ if (sentPtr >= GetFlushRecPtr())
+ WalSndCaughtUp = true;
}
else
{
/*
* If the record we just wanted read is at or beyond the flushed
* point, then we're caught up.
*/
if (logical_decoding_ctx->reader->EndRecPtr >= GetFlushRecPtr())
{

Attached a updated patch. Please review it.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachment Content-Type Size
logical_repl_caught_up_v3.patch application/octet-stream 792 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2017-12-26 10:48:37 Re: pgsql: Add parallel-aware hash joins.
Previous Message Masahiko Sawada 2017-12-26 10:13:22 Re: BUGFIX: standby disconnect can corrupt serialized reorder buffers