hubcio commented on code in PR #3233:
URL: https://github.com/apache/iggy/pull/3233#discussion_r3225762496
##########
core/server/src/shard/tasks/oneshot/config_writer.rs:
##########
@@ -135,5 +135,10 @@ async fn write_config(
info!("Current config written and synced to: {config_path} with all bound
addresses",);
+ #[cfg(feature = "systemd")]
+ if let Err(e) = sd_notify::notify(&[sd_notify::NotifyState::Ready]) {
Review Comment:
`READY=1` is silently lost on any failure of `write_config()` above this
line. the task is registered with `.critical(false)` (around line 32), and `?`
propagates errors from `open` / `write_all_at` / `sync_all` (around lines
110-134) before reaching this notify. so any of {runtime dir missing, disk
full, AppArmor block, read-only fs, custom path override} causes a healthy,
fully-bound server to be killed by systemd's `TimeoutStartSec=` because
`READY=1` never fires. `task_registry` will print error though.
fix: move the notify to right after listener-bind completes (the `if
tcp_ready && quic_ready && http_ready && websocket_ready { break; }` block
earlier in this function), before any fallible disk i/o. readiness is
"listeners are bound", not "config file is fsynced". alternative: set
`.critical(true)` on the oneshot so the task tears down with an error rather
than silently dropping `READY=1`.
the MCP seems fine - `core/ai/mcp/src/main.rs:107` emits `READY=1`
immediately after the synchronous `serve(stdio())` returns with no fallible i/o
in between.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
