core/socket: introduce DeferTrigger= #37505
Conversation
9f2768d to
b34a35f
Compare
d17b37f to
c34b424
Compare
|
Would there be any gain in generalizing this such that it could also be used for timer or path units? |
So, hmm, I'm certainly not against future extensions like this. But OTOH no actual interest has been observed there. The reason why this is strictly necessary for sockets is that they get continuously woken up due to unprocessed IO, which is not a concern for timers/inotify. I can see how this may benefit, however not feeling like adding those immediately. |
5b779d3 to
a50b08f
Compare
1f1b8fa to
c590446
Compare
c590446 to
9069f66
Compare
9069f66 to
ba38ac7
Compare
| @@ -338,6 +338,7 @@ DEFINE_STRING_TABLE_LOOKUP(notify_access, NotifyAccess); | ||
|
|
||
| static const char* const job_mode_table[_JOB_MODE_MAX] = { | ||
| [JOB_FAIL] = "fail", | ||
| [JOB_MEEK] = "meek", |
There was a problem hiding this comment.
maybe call this "additive"? or "augment"
There was a problem hiding this comment.
Not sure. I think "additive" kinda misses the part that this bails on conflict?
ba38ac7 to
ba84305
Compare
eb88d44 to
291e494
Compare
|
Introduced |
291e494 to
6256433
Compare
| won't be disturbed/brought down. Furthermore, if a conflict exists, the socket unit will wait for | ||
| current job queue to complete and potentially defer the activation by then. An upper limit of | ||
| total time to wait can be configured via <varname>DeferTriggerMaxSec=</varname>. The socket unit | ||
| will fail if all jobs have finished or the timeout has been reached but the conflict remains. |
There was a problem hiding this comment.
The condition if all jobs have finished should be dropped, and always wait for the max sec.
| return true; /* changed */ | ||
| } | ||
| if (sd_bus_error_has_name(&error, BUS_ERROR_TRANSACTION_IS_DESTRUCTIVE)) { | ||
| if (!hashmap_isempty(u->manager->jobs)) |
There was a problem hiding this comment.
Now max delay is implemented, then the condition should not be necessary anymore.
| if (s->defer_trigger) { | ||
| r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT_DEREF(s->service), JOB_MEEK, &error, /* ret = */ NULL); | ||
| if (r < 0 && sd_bus_error_has_name(&error, BUS_ERROR_TRANSACTION_IS_DESTRUCTIVE) && | ||
| !hashmap_isempty(UNIT(s)->manager->jobs)) |
|
I really don't understand the back and forth with the job pool being empty part. Should it get dropped, timer would become the "canonical" source of defer, which completely misses the point that the conflicting dependency is what actually matters here, not the time as an extra (optional) safety net. In the specific use case of switch-root/soft-reboot it matters as mentioned, because you can't possibly know how long it would take for the stop phase to complete. Trying to set up a timer as boundary is futile. OTOH it makes perfect sense to bail if there're no jobs, from which point on we just can't expect the conflict to resolve itself anymore. That's precisely why |
Follow-up for 54e1f67
Follow-up for 02229df This applies the change to journal namespace instances too.
Alternative to b50f6db This partially reverts d766c75 The commit naively returned early from socket_enter_running(), which however is quite problematic, as the socket will be woken up over and over again without doing a thing, until we eventually hit Poll/TriggerLimit*=. On top of that it requires hacks to hold the start job for initrd-switch-root.service up. Overall I doubt that is the right approach. Let's instead hook this into our job engine, and try to activate the service again when some other units are stopped. If all installed jobs have been run yet we're still seeing the conflict or the manually selected timeout is reached, fail the socket as before.
systemd-journald.service conflicts with soft-reboot.target, so make sure anything surviving soft-reboot and trying to log to journal doesn't fail the socket units.
|
IOW, this is called |
6256433 to
ee389d5
Compare
Follow-up for #37222 and #37421