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

From: "Hou, Zhijie" <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Muhammad Usama <m(dot)usama(at)gmail(dot)com>
Subject: RE: A new function to wait for the backend exit after termination
Date: 2020-12-03 01:54:33
Message-ID: 2a63dccfc72b44e982627d625092908d@G08CNEXMBPEKD05.g08.fujitsu.local
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-12-03 02:01:34 Re: A new function to wait for the backend exit after termination
Previous Message Michael Paquier 2020-12-03 01:50:40 Re: Deprecate custom encoding conversions