MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="=_5b8fb1ce07a880ac957285622a9ac997"
Content-Transfer-Encoding: 7bit
Date: Thu, 21 Sep 2023 11:18:00 +0900
From: bt23nguyent <bt23nguyent@oss.nttdata.com>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Nathan Bossart <nathandbossart@gmail.com>, Alvaro Herrera
 <alvherre@alvh.no-ip.org>, pgsql-hackers@lists.postgresql.org
Subject: Re: Improve the log message output of basic_archive when
 basic_archive.archive_directory parameter is not set
In-Reply-To: <D3038255-EAB5-4164-A9E3-7967B6738993@yesql.se>
References: <202309151049.dmfsxc5b3wyf@alvherre.pgsql>
 <7035FAFE-7E91-4B31-AA72-75EE82DB3F7B@yesql.se>
 <20230915143827.GA1912920@nathanxps13>
 <85b9e69997bbd5d8302a3a99a2de246d@oss.nttdata.com>
 <D3038255-EAB5-4164-A9E3-7967B6738993@yesql.se>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <18cbd8ca6f5390cda2920644f891a419@oss.nttdata.com>
Archived-At: <https://www.postgresql.org/message-id/18cbd8ca6f5390cda2920644f891a419%40oss.nttdata.com>
Precedence: bulk

--=_5b8fb1ce07a880ac957285622a9ac997
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII;
 format=flowed

On 2023-09-20 21:14, Daniel Gustafsson wrote:
>> On 19 Sep 2023, at 11:21, bt23nguyent <bt23nguyent@oss.nttdata.com> 
>> wrote:
> 
>> Please let me know if you have any further suggestions that I can 
>> improve more.
> 
> +        *logdetail = pstrdup("WAL archiving failed because
> basic_archive.archive_directory is not set");
> 
> Nitpick: detail messages should end with a period per the error message 
> style
> guide [0].
> 

Yes! I totally missed this detail.

> -    archiving will proceed only when it returns 
> <literal>true</literal>.
> +    archiving will proceed only when it returns 
> <literal>true</literal>. The
> +    archiver may also emit the detail explaining how the module is
> not configured
> +    to the sever log if the archive module has any.
> 
> I think this paragraph needs to be updated to include how the returned
> logdetail is emitted, since it currently shows the WARNING without 
> mentioning
> the added detail in case returned.  It would also be good to mention 
> that it
> should be an allocated string which the caller can free.
> 
> --
> Daniel Gustafsson
> 
> [0] https://www.postgresql.org/docs/devel/error-style-guide.html


Thank you for your kind review comment!

I agree with you that this document update is not explanatory enough.
So here is an updated patch.

If there is any further suggestion, please let me know.

Best regards,
Tung Nguyen
--=_5b8fb1ce07a880ac957285622a9ac997
Content-Transfer-Encoding: base64
Content-Type: text/x-diff;
 name=v3-0001-Improve-log-message-output-of-basic-archive.patch
Content-Disposition: attachment;
 filename=v3-0001-Improve-log-message-output-of-basic-archive.patch;
 size=5196

ZGlmZiAtLWdpdCBhL2NvbnRyaWIvYmFzaWNfYXJjaGl2ZS9iYXNpY19hcmNoaXZlLmMgYi9jb250
cmliL2Jhc2ljX2FyY2hpdmUvYmFzaWNfYXJjaGl2ZS5jCmluZGV4IDRkNzhjMzE4NTkuLmRkMGYy
ODE2ZGMgMTAwNjQ0Ci0tLSBhL2NvbnRyaWIvYmFzaWNfYXJjaGl2ZS9iYXNpY19hcmNoaXZlLmMK
KysrIGIvY29udHJpYi9iYXNpY19hcmNoaXZlL2Jhc2ljX2FyY2hpdmUuYwpAQCAtNDgsNyArNDgs
NyBAQCB0eXBlZGVmIHN0cnVjdCBCYXNpY0FyY2hpdmVEYXRhCiBzdGF0aWMgY2hhciAqYXJjaGl2
ZV9kaXJlY3RvcnkgPSBOVUxMOwogCiBzdGF0aWMgdm9pZCBiYXNpY19hcmNoaXZlX3N0YXJ0dXAo
QXJjaGl2ZU1vZHVsZVN0YXRlICpzdGF0ZSk7Ci1zdGF0aWMgYm9vbCBiYXNpY19hcmNoaXZlX2Nv
bmZpZ3VyZWQoQXJjaGl2ZU1vZHVsZVN0YXRlICpzdGF0ZSk7CitzdGF0aWMgYm9vbCBiYXNpY19h
cmNoaXZlX2NvbmZpZ3VyZWQoQXJjaGl2ZU1vZHVsZVN0YXRlICpzdGF0ZSwgY2hhciAqKmxvZ2Rl
dGFpbCk7CiBzdGF0aWMgYm9vbCBiYXNpY19hcmNoaXZlX2ZpbGUoQXJjaGl2ZU1vZHVsZVN0YXRl
ICpzdGF0ZSwgY29uc3QgY2hhciAqZmlsZSwgY29uc3QgY2hhciAqcGF0aCk7CiBzdGF0aWMgdm9p
ZCBiYXNpY19hcmNoaXZlX2ZpbGVfaW50ZXJuYWwoY29uc3QgY2hhciAqZmlsZSwgY29uc3QgY2hh
ciAqcGF0aCk7CiBzdGF0aWMgYm9vbCBjaGVja19hcmNoaXZlX2RpcmVjdG9yeShjaGFyICoqbmV3
dmFsLCB2b2lkICoqZXh0cmEsIEd1Y1NvdXJjZSBzb3VyY2UpOwpAQCAtMTU5LDkgKzE1OSwxNSBA
QCBjaGVja19hcmNoaXZlX2RpcmVjdG9yeShjaGFyICoqbmV3dmFsLCB2b2lkICoqZXh0cmEsIEd1
Y1NvdXJjZSBzb3VyY2UpCiAgKiBDaGVja3MgdGhhdCBhcmNoaXZlX2RpcmVjdG9yeSBpcyBub3Qg
YmxhbmsuCiAgKi8KIHN0YXRpYyBib29sCi1iYXNpY19hcmNoaXZlX2NvbmZpZ3VyZWQoQXJjaGl2
ZU1vZHVsZVN0YXRlICpzdGF0ZSkKK2Jhc2ljX2FyY2hpdmVfY29uZmlndXJlZChBcmNoaXZlTW9k
dWxlU3RhdGUgKnN0YXRlLCBjaGFyICoqbG9nZGV0YWlsKQogewotCXJldHVybiBhcmNoaXZlX2Rp
cmVjdG9yeSAhPSBOVUxMICYmIGFyY2hpdmVfZGlyZWN0b3J5WzBdICE9ICdcMCc7CisJaWYgKGFy
Y2hpdmVfZGlyZWN0b3J5ID09IE5VTEwgfHwgYXJjaGl2ZV9kaXJlY3RvcnlbMF0gPT0gJ1wwJykK
KyAgICB7CisgICAgICAgICpsb2dkZXRhaWwgPSBwc3RyZHVwKCJXQUwgYXJjaGl2aW5nIGZhaWxl
ZCBiZWNhdXNlIGJhc2ljX2FyY2hpdmUuYXJjaGl2ZV9kaXJlY3RvcnkgaXMgbm90IHNldC4iKTsK
KyAgICAgICAgcmV0dXJuIGZhbHNlOworICAgIH0KKyAgICBlbHNlCisgICAgICAgIHJldHVybiB0
cnVlOwogfQogCiAvKgpkaWZmIC0tZ2l0IGEvZG9jL3NyYy9zZ21sL2FyY2hpdmUtbW9kdWxlcy5z
Z21sIGIvZG9jL3NyYy9zZ21sL2FyY2hpdmUtbW9kdWxlcy5zZ21sCmluZGV4IDcwNjQzMDdkOWUu
LmI1OGVkMjM4YjQgMTAwNjQ0Ci0tLSBhL2RvYy9zcmMvc2dtbC9hcmNoaXZlLW1vZHVsZXMuc2dt
bAorKysgYi9kb2Mvc3JjL3NnbWwvYXJjaGl2ZS1tb2R1bGVzLnNnbWwKQEAgLTEwMSw3ICsxMDEs
NyBAQCB0eXBlZGVmIHZvaWQgKCpBcmNoaXZlU3RhcnR1cENCKSAoQXJjaGl2ZU1vZHVsZVN0YXRl
ICpzdGF0ZSk7CiAgICAgYXNzdW1lcyB0aGUgbW9kdWxlIGlzIGNvbmZpZ3VyZWQuCiAKIDxwcm9n
cmFtbGlzdGluZz4KLXR5cGVkZWYgYm9vbCAoKkFyY2hpdmVDaGVja0NvbmZpZ3VyZWRDQikgKEFy
Y2hpdmVNb2R1bGVTdGF0ZSAqc3RhdGUpOwordHlwZWRlZiBib29sICgqQXJjaGl2ZUNoZWNrQ29u
ZmlndXJlZENCKSAoQXJjaGl2ZU1vZHVsZVN0YXRlICpzdGF0ZSwgY2hhciAqKmxvZ2RldGFpbCk7
CiA8L3Byb2dyYW1saXN0aW5nPgogCiAgICAgSWYgPGxpdGVyYWw+dHJ1ZTwvbGl0ZXJhbD4gaXMg
cmV0dXJuZWQsIHRoZSBzZXJ2ZXIgd2lsbCBwcm9jZWVkIHdpdGgKQEAgLTExMiw3ICsxMTIsMTAg
QEAgdHlwZWRlZiBib29sICgqQXJjaGl2ZUNoZWNrQ29uZmlndXJlZENCKSAoQXJjaGl2ZU1vZHVs
ZVN0YXRlICpzdGF0ZSk7CiBXQVJOSU5HOiAgYXJjaGl2ZV9tb2RlIGVuYWJsZWQsIHlldCBhcmNo
aXZpbmcgaXMgbm90IGNvbmZpZ3VyZWQKIDwvc2NyZWVuPgogICAgIEluIHRoZSBsYXR0ZXIgY2Fz
ZSwgdGhlIHNlcnZlciB3aWxsIHBlcmlvZGljYWxseSBjYWxsIHRoaXMgZnVuY3Rpb24sIGFuZAot
ICAgIGFyY2hpdmluZyB3aWxsIHByb2NlZWQgb25seSB3aGVuIGl0IHJldHVybnMgPGxpdGVyYWw+
dHJ1ZTwvbGl0ZXJhbD4uCisgICAgYXJjaGl2aW5nIHdpbGwgcHJvY2VlZCBvbmx5IHdoZW4gaXQg
cmV0dXJucyA8bGl0ZXJhbD50cnVlPC9saXRlcmFsPi4gVGhlCisgICAgYXJjaGl2ZXIgbWF5IGFs
c28gZW1pdCB0aGUgZGV0YWlsIGV4cGxhaW5pbmcgaG93IHRoZSBtb2R1bGUgaXMgbm90IGNvbmZp
Z3VyZWQKKyAgICB0byB0aGUgc2V2ZXIgbG9nIGlmIHRoZSBhcmNoaXZlIG1vZHVsZSBoYXMgYW55
LiBUaGUgZGV0YWlsIG1lc3NhZ2Ugb2YgYXJjaGl2ZQorICAgIG1vZHVsZSBzaG91bGQgYmUgYW4g
YWxsb2NhdGVkIHN0cmluZyB3aGljaCB0aGUgY2FsbGVyIGNhbiBmcmVlLgogICAgPC9wYXJhPgog
ICA8L3NlY3QyPgogCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC9hcmNoaXZlL3NoZWxsX2FyY2hp
dmUuYyBiL3NyYy9iYWNrZW5kL2FyY2hpdmUvc2hlbGxfYXJjaGl2ZS5jCmluZGV4IDE1N2NhNGM3
NTEuLmM5NTdhMThlZTcgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL2FyY2hpdmUvc2hlbGxfYXJj
aGl2ZS5jCisrKyBiL3NyYy9iYWNrZW5kL2FyY2hpdmUvc2hlbGxfYXJjaGl2ZS5jCkBAIC0yMyw3
ICsyMyw3IEBACiAjaW5jbHVkZSAiY29tbW9uL3BlcmNlbnRyZXBsLmgiCiAjaW5jbHVkZSAicGdz
dGF0LmgiCiAKLXN0YXRpYyBib29sIHNoZWxsX2FyY2hpdmVfY29uZmlndXJlZChBcmNoaXZlTW9k
dWxlU3RhdGUgKnN0YXRlKTsKK3N0YXRpYyBib29sIHNoZWxsX2FyY2hpdmVfY29uZmlndXJlZChB
cmNoaXZlTW9kdWxlU3RhdGUgKnN0YXRlLCBjaGFyICoqbG9nZGV0YWlsKTsKIHN0YXRpYyBib29s
IHNoZWxsX2FyY2hpdmVfZmlsZShBcmNoaXZlTW9kdWxlU3RhdGUgKnN0YXRlLAogCQkJCQkJCSAg
IGNvbnN0IGNoYXIgKmZpbGUsCiAJCQkJCQkJICAgY29uc3QgY2hhciAqcGF0aCk7CkBAIC00Myw3
ICs0Myw3IEBAIHNoZWxsX2FyY2hpdmVfaW5pdCh2b2lkKQogfQogCiBzdGF0aWMgYm9vbAotc2hl
bGxfYXJjaGl2ZV9jb25maWd1cmVkKEFyY2hpdmVNb2R1bGVTdGF0ZSAqc3RhdGUpCitzaGVsbF9h
cmNoaXZlX2NvbmZpZ3VyZWQoQXJjaGl2ZU1vZHVsZVN0YXRlICpzdGF0ZSwgY2hhciAqKmxvZ2Rl
dGFpbCkKIHsKIAlyZXR1cm4gWExvZ0FyY2hpdmVDb21tYW5kWzBdICE9ICdcMCc7CiB9CmRpZmYg
LS1naXQgYS9zcmMvYmFja2VuZC9wb3N0bWFzdGVyL3BnYXJjaC5jIGIvc3JjL2JhY2tlbmQvcG9z
dG1hc3Rlci9wZ2FyY2guYwppbmRleCA0NmFmMzQ5NTY0Li4yZmQxZDAzYjA5IDEwMDY0NAotLS0g
YS9zcmMvYmFja2VuZC9wb3N0bWFzdGVyL3BnYXJjaC5jCisrKyBiL3NyYy9iYWNrZW5kL3Bvc3Rt
YXN0ZXIvcGdhcmNoLmMKQEAgLTM5MCw3ICszOTAsOCBAQCBwZ2FyY2hfQXJjaGl2ZXJDb3B5TG9v
cCh2b2lkKQogCQl7CiAJCQlzdHJ1Y3Qgc3RhdCBzdGF0X2J1ZjsKIAkJCWNoYXIJCXBhdGhuYW1l
W01BWFBHUEFUSF07Ci0KKwkJCWNoYXIgICAgICAgKmxvZ2RldGFpbDsKKwkJCQogCQkJLyoKIAkJ
CSAqIERvIG5vdCBpbml0aWF0ZSBhbnkgbW9yZSBhcmNoaXZlIGNvbW1hbmRzIGFmdGVyIHJlY2Vp
dmluZwogCQkJICogU0lHVEVSTSwgbm9yIGFmdGVyIHRoZSBwb3N0bWFzdGVyIGhhcyBkaWVkIHVu
ZXhwZWN0ZWRseS4gVGhlCkBAIC00MTAsMTAgKzQxMSwxMyBAQCBwZ2FyY2hfQXJjaGl2ZXJDb3B5
TG9vcCh2b2lkKQogCiAJCQkvKiBjYW4ndCBkbyBhbnl0aGluZyBpZiBub3QgY29uZmlndXJlZCAu
Li4gKi8KIAkJCWlmIChBcmNoaXZlQ2FsbGJhY2tzLT5jaGVja19jb25maWd1cmVkX2NiICE9IE5V
TEwgJiYKLQkJCQkhQXJjaGl2ZUNhbGxiYWNrcy0+Y2hlY2tfY29uZmlndXJlZF9jYihhcmNoaXZl
X21vZHVsZV9zdGF0ZSkpCisJCQkJIUFyY2hpdmVDYWxsYmFja3MtPmNoZWNrX2NvbmZpZ3VyZWRf
Y2IoYXJjaGl2ZV9tb2R1bGVfc3RhdGUsICZsb2dkZXRhaWwpKQogCQkJewogCQkJCWVyZXBvcnQo
V0FSTklORywKLQkJCQkJCShlcnJtc2coImFyY2hpdmVfbW9kZSBlbmFibGVkLCB5ZXQgYXJjaGl2
aW5nIGlzIG5vdCBjb25maWd1cmVkIikpKTsKKwkJCQkJCShlcnJtc2coImFyY2hpdmVfbW9kZSBl
bmFibGVkLCB5ZXQgYXJjaGl2aW5nIGlzIG5vdCBjb25maWd1cmVkIikpLAorCQkJCQkJKGxvZ2Rl
dGFpbCAhPSBOVUxMKSA/IGVycmRldGFpbCgiJXMiLCBsb2dkZXRhaWwpIDogMCk7CisJCQkJaWYg
KGxvZ2RldGFpbCAhPSBOVUxMKQorCQkJCQlwZnJlZShsb2dkZXRhaWwpOwogCQkJCXJldHVybjsK
IAkJCX0KIApkaWZmIC0tZ2l0IGEvc3JjL2luY2x1ZGUvYXJjaGl2ZS9hcmNoaXZlX21vZHVsZS5o
IGIvc3JjL2luY2x1ZGUvYXJjaGl2ZS9hcmNoaXZlX21vZHVsZS5oCmluZGV4IDY3OWNlNWE2ZGIu
LjQzYmQwZmY0YzIgMTAwNjQ0Ci0tLSBhL3NyYy9pbmNsdWRlL2FyY2hpdmUvYXJjaGl2ZV9tb2R1
bGUuaAorKysgYi9zcmMvaW5jbHVkZS9hcmNoaXZlL2FyY2hpdmVfbW9kdWxlLmgKQEAgLTM2LDcg
KzM2LDcgQEAgdHlwZWRlZiBzdHJ1Y3QgQXJjaGl2ZU1vZHVsZVN0YXRlCiAgKiBhcmNoaXZlIG1v
ZHVsZXMgZG9jdW1lbnRhdGlvbi4KICAqLwogdHlwZWRlZiB2b2lkICgqQXJjaGl2ZVN0YXJ0dXBD
QikgKEFyY2hpdmVNb2R1bGVTdGF0ZSAqc3RhdGUpOwotdHlwZWRlZiBib29sICgqQXJjaGl2ZUNo
ZWNrQ29uZmlndXJlZENCKSAoQXJjaGl2ZU1vZHVsZVN0YXRlICpzdGF0ZSk7Cit0eXBlZGVmIGJv
b2wgKCpBcmNoaXZlQ2hlY2tDb25maWd1cmVkQ0IpIChBcmNoaXZlTW9kdWxlU3RhdGUgKnN0YXRl
LCBjaGFyICoqbG9nZGV0YWlsKTsKIHR5cGVkZWYgYm9vbCAoKkFyY2hpdmVGaWxlQ0IpIChBcmNo
aXZlTW9kdWxlU3RhdGUgKnN0YXRlLCBjb25zdCBjaGFyICpmaWxlLCBjb25zdCBjaGFyICpwYXRo
KTsKIHR5cGVkZWYgdm9pZCAoKkFyY2hpdmVTaHV0ZG93bkNCKSAoQXJjaGl2ZU1vZHVsZVN0YXRl
ICpzdGF0ZSk7
--=_5b8fb1ce07a880ac957285622a9ac997--