Bug#961511: [Pkg-xen-devel] Bug#961511: [PATCH] d/xen-utils-common.xen.init: disable oom killer for xenstored
On Tue, Sep 22, 2020 at 02:39:09PM +0200, Hans van Kranenburg wrote: > How did you test it and how did you get a working process without the --? By reading the man page, noticing there was no mention of "--" and then trying `choom -n +5 sleep 5` and found that worked. When you sent this message I checked and GNU `sleep` does have "--version", thus I tried `choom -n +5 sleep 5 --version` and found *that* failed. A "--" seemed natural, but documentation omitting crucial details is a problem. Never mind. Nice find, I did at one point have the oom-killer get the wrong process and saw *problems*. -- (\___(\___(\__ --=> 8-) EHM <=-- __/)___/)___/) \BS (| ehem+sig...@m5p.com PGP 87145445 |) / \_CS\ | _ -O #include O- _ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Bug#961511: [Pkg-xen-devel] Bug#961511: [PATCH] d/xen-utils-common.xen.init: disable oom killer for xenstored
notfixed 961511 xen/4.14.0-1~exp1 thanks Right... so in the end I made an off-by-one error while rebasing and totally lost that commit. It's not actually in 4.14.0-1~exp1 now. That's bad. On 9/21/20 3:50 AM, Elliott Mitchell wrote: > This is fun. Actually isn't too difficult to trigger, simply slowly > reduce the memory Xen allocates to Dom0 and eventually the oom-killer is > likely to trigger (having tried to shrink Dom0 as far as possible, > believe me, I know). I had been wondering which of the Xen daemons could > be safely restarted since it is handy to restart daemons instead of whole > machine for security updates... > > Interestingly running `xenstored --help` mentions: > -I, --internal-db store database in memory, not on disk > > There is a run/xenstored/tdb file so I end up wondering if newer versions > are in fact storing everything in a file and restarting isn't so bad. Not by default, and I don't know if it's actually considered best practice. I could not find any info about this yet. I suspect it's not recommended. oxenstored has the following option in /etc/xen/oxenstored.conf: # Activate filed base backend persistent = false When enabling this, the file /run/xenstored/db gets rewritten a lot and I also see it's out of sync with what's in xenstore-ls after doing some things. So, it might me inconsistent when the process is oom-killed. > The patch switches the arguments from: > --exec "$try_xenstored" -- ... > to: > --exec /usr/bin/choom -- -n -1000 "$try_xenstored" -- ... > > I'm pretty sure start-stop-daemon is consuming the "--" and the second > "--" shouldn't be there. Well, I tested it and found out that it's needed... -# start-stop-daemon --start \ --pidfile "/run/xenstore.pid" \ --exec /usr/bin/choom -- -n -1000 \ /usr/lib/xen-4.14/bin/oxenstored --pid-file "/run/xenstore.pid" /usr/bin/choom: unrecognized option '--pid-file' Try 'choom --help' for more information. -# start-stop-daemon --start \ --pidfile "/run/xenstore.pid" \ --exec /usr/lib/xen-4.14/bin/oxenstored --test Would start /usr/lib/xen-4.14/bin/oxenstored . and with the extra separator: -# start-stop-daemon --start \ --pidfile "/run/xenstore.pid" \ --exec /usr/bin/choom -- -n -1000 \ /usr/lib/xen-4.14/bin/oxenstored -- --pid-file "/run/xenstore.pid" -# grep . /proc/$(pidof /usr/lib/xen-4.14/bin/oxenstored)/oom_* /proc/363043/oom_adj:-17 /proc/363043/oom_score:0 /proc/363043/oom_score_adj:-1000 -# cat /proc/$(pidof /usr/lib/xen-4.14/bin/oxenstored)/cmdline /usr/lib/xen-4.14/bin/oxenstored--pid-file/run/xenstore.pid How did you test it and how did you get a working process without the --? Hans
Bug#961511: [PATCH] d/xen-utils-common.xen.init: disable oom killer for xenstored
This is fun. Actually isn't too difficult to trigger, simply slowly reduce the memory Xen allocates to Dom0 and eventually the oom-killer is likely to trigger (having tried to shrink Dom0 as far as possible, believe me, I know). I had been wondering which of the Xen daemons could be safely restarted since it is handy to restart daemons instead of whole machine for security updates... Interestingly running `xenstored --help` mentions: -I, --internal-db store database in memory, not on disk There is a run/xenstored/tdb file so I end up wondering if newer versions are in fact storing everything in a file and restarting isn't so bad. The patch switches the arguments from: --exec "$try_xenstored" -- ... to: --exec /usr/bin/choom -- -n -1000 "$try_xenstored" -- ... I'm pretty sure start-stop-daemon is consuming the "--" and the second "--" shouldn't be there. -- (\___(\___(\__ --=> 8-) EHM <=-- __/)___/)___/) \BS (| ehem+sig...@m5p.com PGP 87145445 |) / \_CS\ | _ -O #include O- _ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Bug#961511: [PATCH] d/xen-utils-common.xen.init: disable oom killer for xenstored
~Hans van Kranenburg writes ("[PATCH] d/xen-utils-common.xen.init: disable oom killer for xenstored"): > In case of oom killer terminating some process, we'd rather not see > xenstored go. Xenstored has an in-memory database, and when starting the > process again, it would be empty, which is very inconvenient. Xenstored > should already score quite low and have a fairly low memory footprint, > but according to the user report, it happened. > > Closes: #961511 > Suggested-by: Samuel Thibault > Signed-off-by: Hans van Kranenburg Acked-by: Ian Jackson
Bug#961511: [PATCH] d/xen-utils-common.xen.init: disable oom killer for xenstored
tag -1 + pending thanks On 9/7/20 12:40 PM, Ian Jackson wrote: > ~Hans van Kranenburg writes ("[PATCH] d/xen-utils-common.xen.init: disable > oom killer for xenstored"): >> In case of oom killer terminating some process, we'd rather not see >> xenstored go. Xenstored has an in-memory database, and when starting the >> process again, it would be empty, which is very inconvenient. Xenstored >> should already score quite low and have a fairly low memory footprint, >> but according to the user report, it happened. >> >> Closes: #961511 >> Suggested-by: Samuel Thibault >> Signed-off-by: Hans van Kranenburg > > Acked-by: Ian Jackson Thanks, added. Hans
Bug#961511: [PATCH] d/xen-utils-common.xen.init: disable oom killer for xenstored
In case of oom killer terminating some process, we'd rather not see xenstored go. Xenstored has an in-memory database, and when starting the process again, it would be empty, which is very inconvenient. Xenstored should already score quite low and have a fairly low memory footprint, but according to the user report, it happened. Closes: #961511 Suggested-by: Samuel Thibault Signed-off-by: Hans van Kranenburg --- Cc: Ian Jackson --- This is in my knorrie/4.14-extra branch now. I think we should do this. --- debian/xen-utils-common.xen.init | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/debian/xen-utils-common.xen.init b/debian/xen-utils-common.xen.init index 54aaba89d320..2a4c09fa3f71 100644 --- a/debian/xen-utils-common.xen.init +++ b/debian/xen-utils-common.xen.init @@ -226,7 +226,8 @@ xenstored_start() eval "try_xenstored=\$$try_xenstored_var" if [ -x $try_xenstored ]; then if start-stop-daemon --start --quiet \ - --pidfile "$XENSTORED_PIDFILE" --exec "$try_xenstored" -- \ + --pidfile "$XENSTORED_PIDFILE" \ + --exec /usr/bin/choom -- -n -1000 "$try_xenstored" -- \ $XENSTORED_ARGS --pid-file "$XENSTORED_PIDFILE"; then started_xenstored=$try_xenstored break -- 2.20.1