From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | sawada(dot)mshk(at)gmail(dot)com |
Cc: | peter(dot)eisentraut(at)2ndquadrant(dot)com, petr(dot)jelinek(at)2ndquadrant(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Interval for launching the table sync worker |
Date: | 2017-04-28 08:26:24 |
Message-ID: | 20170428.172624.06342660.horiguchi.kyotaro@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
At Fri, 28 Apr 2017 10:20:48 +0900, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote in <CAD21AoBY9UvS9QLrmaENGBGfQKOfGkGaLm=uYH24gmf-6CAoiw(at)mail(dot)gmail(dot)com>
> On Fri, Apr 28, 2017 at 4:00 AM, Peter Eisentraut
> <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> > On 4/27/17 06:47, Petr Jelinek wrote:
> >> One thing I am missing in your patch however is cleanup of entries for
> >> relations that finished sync. I wonder if it would be enough to just
> >> destroy the hash when we get to empty list.
> >
> > I had omitted that because the amount of memory "leaked" is not much,
> > but I guess it wouldn't hurt to clean it up.
> >
> > How about the attached?
> >
This seems rasonable enough.
> Thank you for updating patch!
>
> + /*
> + * Clean up the hash table when we're done with all tables (just to
> + * release the bit of memory).
> + */
> + else if (!table_states && last_start_times)
> + {
>
> Isn't it better to use != NIL instead as follows?
>
> else if (table_state != NIL && last_start_times)
Definitely!, but maybe should be reverse condition.
- if (table_states && !last_start_times)
+ if (table_states != NIL && !last_start_times)
===
- else if (!table_states && last_start_times)
+ else if (table_states == NIL && last_start_times)
reagrds,
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | henry_boehlert | 2017-04-28 08:28:18 | BUG #14634: On Windows pg_basebackup should write tar to stdout in binary mode |
Previous Message | Rajkumar Raghuwanshi | 2017-04-28 08:11:48 | Re: Declarative partitioning - another take |