RE: Parallel copy

From: "Hou, Zhijie" <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, "Andres Freund" <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, Ants Aasma <ants(at)cybertec(dot)at>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, "Alastair Turner" <minion(at)decodable(dot)me>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Subject: RE: Parallel copy
Date: 2020-10-28 12:06:34
Message-ID: 37f541303fe747d2aa23fbcbbad12c25@G08CNEXMBPEKD05.g08.fujitsu.local
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

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?

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

4.
+ if (pcdata->worker_line_buf_count)

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

Best regards,
houzj

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2020-10-28 12:14:33 Re: [PATCH] Add features to pg_stat_statements
Previous Message Sergei Kornilov 2020-10-28 12:02:33 Re: Allow some recovery parameters to be changed with reload