RE: A new function to wait for the backend exit after termination

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

From: "Hou, Zhijie" <houzj.fnst@cn.fujitsu.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Muhammad Usama <m.usama@gmail.com>
Date: 2020-12-03T01:54:33Z
Lists: pgsql-hackers
Hi

I take a look into the patch, and here some comments.

1.
+
+	ereport(WARNING,
+			(errmsg("could not wait for the termination of the backend with PID %d within %ld milliseconds",
+					pid, timeout)));
+

The code use %ld to print int64 type.
How about use INT64_FORMAT, which looks more appropriate. 

2.
+	if (timeout <= 0)
+	{
+		ereport(WARNING,
+				(errmsg("timeout cannot be negative or zero: %ld", timeout)));
+		PG_RETURN_BOOL(r);
+	}

The same as 1.

3.
+pg_terminate_backend_and_wait(PG_FUNCTION_ARGS)
+{
+	int 	pid = PG_GETARG_DATUM(0);

+pg_wait_backend(PG_FUNCTION_ARGS)
+{
+	int		pid = PG_GETARG_INT32(0);

The code use different macro to get pid,
How about use PG_GETARG_INT32(0) for each one.


I changed the status to 'wait on anthor'.
The others of the patch LGTM, 
I think it can be changed to Ready for Committer again, when this comment is confirmed.


Best regards,
houzj






Commits

  1. Copy-edit text for the pg_terminate_backend() "timeout" parameter.

  2. Remove pg_wait_for_backend_termination().

  3. Add functions to wait for backend termination

  4. Fix typo in comment.