Обсуждение: Re: [COMMITTERS] pgsql: Make walsender more responsive.

On Mon, Jul 2, 2012 at 10:49 PM, Robert Haas <rhaas@postgresql.org> wrote:
> Make walsender more responsive.
>
> Per testing by Andres Freund, this improves replication performance
> and reduces replication latency and latency jitter.  I was a bit
> concerned about moving more work into XLogInsert, but testing seems
> to show that it's not a problem in practice.
>
> Along the way, improve comments for WaitLatchOrSocket.

This commit makes the synchronous replication slow down very much
when wal_sync_method is set to open_sync or open_datasync. I think
the attached patch needs to be applied.

+#define WalSndWakeupProcessRequests()        \
+    do                                        \
+    {                                        \
+        if (wake_wal_senders)                \
+        {                                    \
+            wake_wal_senders = false;        \
+            if (max_wal_senders > 0)        \
+                WalSndWakeup();                \
+        }                                    \
+    } while (0)

I'm not sure it's really worth doing, but isn't it good idea to test
max_wal_sender > 0 first to eliminate any CPU cycle in non replication case?

Regards,

--
Fujii Masao

On Monday, July 02, 2012 07:19:19 PM Fujii Masao wrote:
> On Mon, Jul 2, 2012 at 10:49 PM, Robert Haas <rhaas@postgresql.org> wrote:
> > Make walsender more responsive.
> > 
> > Per testing by Andres Freund, this improves replication performance
> > and reduces replication latency and latency jitter.  I was a bit
> > concerned about moving more work into XLogInsert, but testing seems
> > to show that it's not a problem in practice.
> > 
> > Along the way, improve comments for WaitLatchOrSocket.
> 
> This commit makes the synchronous replication slow down very much
> when wal_sync_method is set to open_sync or open_datasync. I think
> the attached patch needs to be applied.
Hm. Yes, definitely. No idea why I placed the call there, sorry.

Thats how synchronous_write=off behaved generally till the recent (simple) fix 
btw ;)

> +#define WalSndWakeupProcessRequests()        \
> +    do                                        \
> +    {                                        \
> +        if (wake_wal_senders)                \
> +        {                                    \
> +            wake_wal_senders = false;        \
> +            if (max_wal_senders > 0)        \
> +                WalSndWakeup();                \
> +        }                                    \
> +    } while (0)
> 
> I'm not sure it's really worth doing, but isn't it good idea to test
> max_wal_sender > 0 first to eliminate any CPU cycle in non replication
> case?
I think the difference is ignorable. wake_wal_senders probably has better 
cache locality but is set to true more often, but not that often...

Thanks,

Andres
-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

On Mon, Jul 2, 2012 at 1:53 PM, Andres Freund <andres@2ndquadrant.com> wrote:
>> This commit makes the synchronous replication slow down very much
>> when wal_sync_method is set to open_sync or open_datasync. I think
>> the attached patch needs to be applied.
> Hm. Yes, definitely. No idea why I placed the call there, sorry.

Committed.

>> +#define WalSndWakeupProcessRequests()                \
>> +     do                                                                              \
>> +     {                                                                               \
>> +             if (wake_wal_senders)                           \
>> +             {                                                                       \
>> +                     wake_wal_senders = false;               \
>> +                     if (max_wal_senders > 0)                \
>> +                             WalSndWakeup();                         \
>> +             }                                                                       \
>> +     } while (0)
>>
>> I'm not sure it's really worth doing, but isn't it good idea to test
>> max_wal_sender > 0 first to eliminate any CPU cycle in non replication
>> case?
> I think the difference is ignorable. wake_wal_senders probably has better
> cache locality but is set to true more often, but not that often...

I was wondering if we shouldn't do this as:

if (max_wal_senders > 0 && wake_wal_senders)   WalSndWakeup();

....and then put wake_wal_senders = false into WalSndWakeup().

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company