On Fri, 18 Mar 2016 20:40:16 +0300
Dmitry Ivanov <d.ivanov@postgrespro.ru> wrote:
> Hi Constantin,
>
> I did a quick review of your patch, and here are my comments:
>
> - This patch applies cleanly to the current HEAD (61d2ebdbf91).
>
> - Code compiles without warnings.
>
> - Currently there's no documentation regarding parallel gin build
> feature and provided GUC variables.
>
> - Built indexes work seem to work normally.
>
>
> Performance
> -----------
>
> I've made a few runs on my laptop (i7-4710HQ, default
> postgresql.conf), here are the results:
>
> workers avg. time (s)
> 0 412
> 4 133
> 8 81
>
> Looks like 8 workers & a backend do the job 5x times faster than a
> sole backend, which is good!
>
>
> Code style
> ----------
>
> There are some things that you've probably overlooked, such as:
>
> > task->heap_oid = heap->rd_id;
> > task->index_oid = index->rd_id;
>
> You could replace direct access to 'rd_id' field with the
> RelationGetRelid macro.
>
> > static void ginDumpEntry(GinState *ginstate,
> > volatile
> > WorkerResult *r
>
> Parameter 'r' is unused, you could remove it.
>
> Some of the functions and pieces of code that you've added do not
> comply to the formatting conventions, e. g.
>
> > static int claimSomeBlocks(...
> > static GinEntryStack *pushEntry(
>
> >> // The message consists of 2 or 3 parts. iovec allows us to send
> >> them as
>
> etc.
>
> Keep up the good work!
Hi Dmitry,
Thank you for the review. I am working on a new version, that will fix
the code style and also involve backend into the process as one of the
workers.
