Re: Parallel copy

On Wed, Oct 28, 2020 at 5:36 PM Hou, Zhijie <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com>
wrote:
>
> Hi
>
> I found some issue in v9-0002
>
> 1.
> +
> + elog(DEBUG1, "[Worker] Processing - line position:%d, block:%d,
unprocessed lines:%d, offset:%d, line size:%d",
> + write_pos, lineInfo->first_block,
> +
pg_atomic_read_u32(&data_blk_ptr->unprocessed_line_parts),
> + offset, pg_atomic_read_u32(&lineInfo->line_size));
> +
>
> write_pos or other variable to be printed here are type of uint32, I
think it'better to use '%u' in elog msg.
>

Modified it.

> 2.
> + * line_size will be set. Read the line_size again to be
sure if it is
> + * completed or partial block.
> + */
> + dataSize = pg_atomic_read_u32(&lineInfo->line_size);
> + if (dataSize)
>
> It use dataSize( type int ) to get uint32 which seems a little dangerous.
> Is it better to define dataSize uint32 here?
>

Modified it.

> 3.
> Since function with 'Cstate' in name has been changed to 'CState'
> I think we can change function PopulateCommonCstateInfo as well.
>

Modified it.

> 4.
> + if (pcdata->worker_line_buf_count)
>
> I think some check like the above can be 'if (xxx > 0)', which seems
easier to understand.

Modified it.

Thanks for the comments, these issues are fixed in v10 patch posted at [1]
[1]
https://www.postgresql.org/message-id/CALDaNm05FnA-ePvYV_t2%2BWE_tXJymbfPwnm%2Bkc9y1iMkR%2BNbUg%40mail.gmail.com

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com