Re: Documentation Edits for pg_createsubscriber - Mailing list pgsql-hackers
| From | David G. Johnston |
|---|---|
| Subject | Re: Documentation Edits for pg_createsubscriber |
| Date | |
| Msg-id | CAKFQuwZ_di+FZ=jJPY-RaU0QND9MAUAioy0xX9oxJ72PkERUBw@mail.gmail.com Whole thread Raw |
| In response to | RE: Documentation Edits for pg_createsubscriber ("Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>) |
| Responses |
RE: Documentation Edits for pg_createsubscriber
|
| List | pgsql-hackers |
Thank you for the quick reply.
On Mon, Mar 10, 2025 at 7:56 PM Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote:
Dear David,
> The main thing I noted is that our synopsis is simply wrong regarding the
> optionality of the --database option. I went with a fix that included only
> showing the two mandatory options and adding a paragraph to usage explaining
> that --database exists and how to use it. Our existing example also covers
> that common usage.
According to the code, -D and -P is anyway needed, but -d can be omitted when -P
contians dbname option. And you wanted to follow the rule, right?
IIUC shorter synopsis is always welcomed.
I'm having second thoughts here a bit as I absorb more. I'm not really sure why we added --publication-name etc... but I was giving them more weight than is probably needed. They don't seem like something a typical user would care about. I'll probably add back -d but leave mention of all the long-form option names out still. Or, my other thought, two lines of commands and explain the single-database and multi-database mechanics that distinguish them in the description.
> It took me a bit, and reading implementation code, to understand what was meant
> by "Additional recovery parameters are added to avoid ...". I decided that
> detail seemed unnecessary for the user-facing documentation - supposedly they
> will never see the recovery file if everything works OK (and if they do the
> various empty string settings should be reasonably self-evident). If we do
> want to keep it I'll try to find a better wording.
To confirm; users can run SHOW command after the conversion and they can see
our setting. Do you think it is a user-facing?
And they will see empty strings (or blanks or whatever the defaults are) for everything except LSN, like they always do, and not realize or care that our code generated file actually contained lines for them. (I haven't actually experimented here though.) But in normal use they have no reason to inspect the state data generated by this process so which it is accessible as a matter of design it is not practically user-facing for a user of this application. The only state they need to manage is that their WAL retention for their standby is large enough to execute the needed point-in-time recovery.
Here are my comments. (I ran Grammarly to check your sentences)
01.
Your patch breaks 80-column-per-line rule. Please check it.
Ok, though it seems like a very loosely enforced one in the documentation.
02.
```
+ The minimal command shown above...
```
"shown" can be removed.
ok
03.
```
+ then created using auto-generated names, which can be overriden
```
s/overriden/overridden/.
ok
04.
```
+ using the corresponding long-form command-line options, once each per database.
```
"," and "each" can be removed.
I'll ponder this one and explain if I leave it as-is.
05.
```
+ does not perform an initial data copy. Instead it relies on a LSN-based
```
Latter sentence can be "Inseted, it relies on an LSN-based..."
probably, though I have the vague impression that this entire page has too many unneeded commas right now. But not obviously so.
06.
```
+ <para>
+ Note, the configuration setting <literal>port</literal> in this file is effectively ignored
+ as <application>pg_createsubscriber</application> always specifies the target listening port
+ on the <application>pg_ctl</application> command-line.
+ Use the <option>-p</option> option to specify which port the target server should listen on.
</para>
```
Can you clarify the <literal>port</literal> in the config file is ignored only
while the pg_createsuscriber commands?
I'm not following what you want me to do here? This note only applies while pg_createsubscriber manages the target service which the first second basically states directly.
07.
```
+ <application>pg_createsubscriber</application> is designed have full and
```
s/is designed have/is designed to have/.
yep
David J.
pgsql-hackers by date: