On 28.04.21 14:20, Oguz Bektas wrote: > On Wed, Apr 28, 2021 at 01:51:51PM +0200, Thomas Lamprecht wrote: >> On 28.04.21 13:42, Oguz Bektas wrote: >>>>> + >>>>> + LISTEN_IP="fe80::d8ee:34ff:fe37:4579%vmbr0" >>>>> + >>>>> +After the change you have to restart `pveproxy` for it to take effect: >>>> >>>> I'd specifically state that a reload is not enough and then add a small >>>> warning that >>>> a restart can stop some existing workers (not all, but e.g., shell >>>> connection is stopped >>>> and reconnected which may loose info on CTs without a screen/tmux instance >>>> running). >>>> Also, there's a short time window where no new connections are accepted >>>> IIRC (albeit >>>> I was the one fixing that for reload it's been to long since then, so not >>>> sure anymore) >>> >>> i think the phrasing "you have to restart" already emphasizes this, >>> adding too many warnings or notes would just confuse users in my >>> opinion. >> >> No, it's clear that something needs to be restarted, but "restart" is a >> general >> overused term which can mean lots of things (even reboot for some). >> >>> >>> though i don't see any harm in making the **restart** bold in that >>> sentence and adding that small warning about possible connection drop. >> >> as said, restart is often used for the general semantic thing, be it reload >> or restart, >> so this is really not clear. > > why is it not clear? there's not a single instance of 'systemctl reload' > command in the documentation, and in instances where a service restart > is required, we specify 'systemctl restart' without any mention, e.g., > in pvenode.adoc when setting up certificates) > > so i don't see why users would imagine to 'reload' the service instead > of 'restart' when it's clearly written in bold, and the command is > right beneath the explanation... > >> The gui also only triggers a reload, IIRC (pls. check) >> and thus "restarting" (it's named that way there) from there would not help. > > yes, on the button it says "restart" but the tasklog says "reload". > so to me that sounds like a mislabeled button with wrong/lax use > of "restart" instead of the correct "reload".
I *always* use try-reload-or-restart, as I want to avoid service disruption, as most people actually running relevant systems do... What I surely do not is reading all the docs, comparing how often reload vs. restart is used and then try to read into that and conclude something from that... I *never* saw anybody complaining about an additional note of some side-effects that may come unexpected, but I saw quite some situations where there was no such note and user run into issues, I do not suggest such changes just for the hell of it... > >> >> You just need to write it in such a way that it is not confusing, then it is >> not >> a problem. > > there's really nothing confusing IMO (besides the button). but if you > insist i will send another patch with the changes you recommended. You said you would write it in a confusing way when adding an explicit note, so I suggest doing it in a not confusing manner ;) > > some possible alternative phrasing i'd suggest: > > ===== > After the change you have to **restart** `pveproxy` and `spiceproxy` for > it to take effect (**reload** or restart from GUI does not suffice): > systemctl restart pveproxy spiceproxy > ===== > > or much simpler and less confusing: > ===== > Run the following command to restart the proxy servers and apply the change: > systemctl restart pveproxy spiceproxy > ===== > > with a note about the connection drop during restart. > I applied the fixes, the notes and another small grammar style fix for the comma after "Similarly," myself, seemed to be the faster way to resolve this nicely.. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
