Horton

RE: Parallel copy

Hou, Zhijie <houzj.fnst@cn.fujitsu.com>

View thread
From: "Hou, Zhijie" <houzj.fnst@cn.fujitsu.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Rafia Sabih <rafia.pghackers@gmail.com>, Ashutosh Sharma <ashu.coek88@gmail.com>, "Andres Freund" <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>, Ants Aasma <ants@cybertec.at>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, "Alastair Turner" <minion@decodable.me>, Thomas Munro <thomas.munro@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Heikki Linnakangas <hlinnaka@iki.fi>
Date: 2020-10-28T12:06:34Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow WaitLatch() to be used without a latch.

  2. Add %P to log_line_prefix for parallel group leader

  3. Include replication origins in SQL functions for commit timestamp

  4. Avoid useless buffer allocations during binary COPY FROM.

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