Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-29 Thread Michael Paquier
On Wed, Jul 28, 2021 at 10:28:13AM -0400, Robert Haas wrote: > I think all of these are reasonable fixes. In the case of > pg_basebackup, a chmod() failure doesn't necessarily oblige us to give > up and exit; we could presumably still write the data. But it may be > best to give up and exit. The

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-28 Thread Robert Haas
On Tue, Jul 27, 2021 at 11:18 PM Michael Paquier wrote: > I have been looking at that. Here are some findings: > - In pg_archivecleanup, CleanupPriorWALFiles() does not bother > exit()'ing with an error code. Shouldn't we worry about reporting > that correctly? It seems strange to not inform

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-28 Thread Fabien COELHO
[...] Thoughts? For pgbench it is definitely ok to add the exit. For others the added exits look reasonable, but I do not know them intimately enough to be sure that it is the right thing to do in all cases. All that does not seem to enter into the category of things worth

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-27 Thread Michael Paquier
On Tue, Jul 27, 2021 at 08:53:52AM +0900, Michael Paquier wrote: > On Mon, Jul 26, 2021 at 03:35:29PM -0400, Alvaro Herrera wrote: >> (In reality we cannot literally just exit(1) in pg_log_fatal(), because >> there are quite a few places that do some other thing after the log >> call and before

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-27 Thread Daniel Gustafsson
> On 27 Jul 2021, at 01:53, Michael Paquier wrote: > ..and I also recall that this point has been discussed, where the conclusion > was that the logging should never exit() directly. That's a characteristic of the API which still holds IMO. If we want something, it's better to have an

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-27 Thread Michael Paquier
On Tue, Jul 27, 2021 at 11:45:07AM +0200, Fabien COELHO wrote: > Sure. Then what should be the expected usage of fatal? Doc says: > > * Severe errors that cause program termination. (One-shot programs may > * chose to label even fatal errors as merely "errors". The distinction > * is up

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-27 Thread Fabien COELHO
Hello, I do not understand your disagreement. Do you disagree about the expected>> semantics of fatal? Then why provide fatal if it should not be used? What is the expected usage of fatal? I disagree about the fact that pgbench uses pg_log_fatal() in ways that other binaries don't do.

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-27 Thread Michael Paquier
On Tue, Jul 27, 2021 at 06:36:15AM +0200, Fabien COELHO wrote: > I do not understand your disagreement. Do you disagree about the expected > semantics of fatal? Then why provide fatal if it should not be used? > What is the expected usage of fatal? I disagree about the fact that pgbench uses

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-26 Thread Fabien COELHO
Bonjour Michaël-san, The semantics for fatal vs error is that an error is somehow handled while a fatal is not. If the log message is followed by an cold exit, ISTM that fatal is the right call, and I cannot help if other commands do not do that. ISTM more logical to align other commands to

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-26 Thread Michael Paquier
On Mon, Jul 26, 2021 at 03:35:29PM -0400, Alvaro Herrera wrote: > On 2021-Jul-26, Fabien COELHO wrote: >> The semantics for fatal vs error is that an error is somehow handled while a >> fatal is not. If the log message is followed by an cold exit, ISTM that >> fatal is the right call, and I cannot

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-26 Thread Alvaro Herrera
On 2021-Jul-26, Fabien COELHO wrote: > > - pgbench and pg_verifybackup make use of pg_log_fatal() to report > > some failures in code paths dedicated to command-line options, which > > is inconsistent with all the other tools that use pg_log_error(). > > The semantics for fatal vs error is that

Re: Some code cleanup for pgbench and pg_verifybackup

2021-07-26 Thread Fabien COELHO
Bonjour Michaël, My 0.02€: - pgbench has its own parsing routines for int64 and double, with an option to skip errors. That's not surprising in itself, but, for strtodouble(), errorOK is always true, meaning that the error strings are dead. Indeed. However, there are "atof" calls for

Some code cleanup for pgbench and pg_verifybackup

2021-07-26 Thread Michael Paquier
Hi all, While looking at the code areas of $subject, I got surprised about a couple of things: - pgbench has its own parsing routines for int64 and double, with an option to skip errors. That's not surprising in itself, but, for strtodouble(), errorOK is always true, meaning that the error