Re: [HACKERS] Interval for launching the table sync worker

At Fri, 28 Apr 2017 10:20:48 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in
<CAD21AoBY9UvS9QLrmaENGBGfQKOfGkGaLm=uYH24gmf-6CAoiw@mail.gmail.com>
> On Fri, Apr 28, 2017 at 4:00 AM, Peter Eisentraut
> <peter.eisentraut@2ndquadrant.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