netatalk: start cnid_metad only when a volume uses the dbd scheme - #3269
Conversation
|
@rdmark ready for review |
rdmark
left a comment
There was a problem hiding this comment.
this is a nice improvement! one more step towards completely decoupling from the old dbd structure
not strictly speaking a build system constraint, rather that root privileges are required to run the cnid_metad / cnid_dbd daemon combo so I decided not to use it for the afpd tests we could easily add support for the mysql backend in the afpd tests if it's valuable |
The master started cnid_metad whenever the dbd backend was compiled in, regardless of whether any volume has 'cnid scheme = dbd'. All backends compile by default and the default scheme is sqlite (in-process), so a stock install ran an idle cnid_metad/cnid_dbd pair that nothing talks to. Decide from the configuration instead: start cnid_metad at startup only when a volume resolves to dbd, re-evaluate on SIGHUP (starting or stopping it as volumes change), and demote a crash-restart to no-op when dbd is no longer in use. The check also resolves the [Homes] section, which the master's volume load skips (homes volumes are instantiated per-user at login), and falls back to starting the daemon when the volume load itself failed. Builds without the dbd backend are unchanged.
c000f3c to
73d230c
Compare
|
|
@rdmark feedback resolved |
📊 Performance DashboardCommit: 🔥 Spectest (AFP 3.4) - FlameGraphNetatalk Code-time: 2.4% · Runtime: 61s · Stacks: 1638 🔥 Click the preview to open the interactive flamegraph (zoom + search). 🔝 Top 10 leaf functions
📈 Speedtest (AFP 3.4) - PerfGraphPeak Read: 6864 MB/s (-4.2% vs hist avg 7165.5 MB/s; min 5963 / max 9526 over 27 PRs) 🔝 Throughputs per operation (vs. historical average)
⏱️ Lantest (AFP 3.4) - LatencyGraphAvg total runtime: 2906 ms (-31.6% vs hist avg 4249.2 ms; min 2122 / max 5208 over 27 PRs) 🐢 All operations (avg runtime, in test order, vs. historical average)
Run baseline: median op-test delta -28.5%, MAD 5.3%. Adj Δ% shifts each delta by the median; standouts ≥5% in bold. A large MAD means the run did not move uniformly — read the adjusted column with caution. |







Fixes #3268.
Problem
The netatalk master started
cnid_metadwhenever the dbd backend was merely compiled in (CNID_BACKEND_DBD), regardless of whether any volume hascnid scheme = dbd. All three backends compile by default and the default scheme is sqlite (in-process), so a stock install ran an idlecnid_metad/cnid_dbdpair that nothing talks to.Design
The master already has the machinery: it loads volumes before starting services and re-loads them on SIGHUP, and its timer re-launches any service whose pid sentinel is
NETATALK_SRV_NEEDED. The change makes the needed/optional decision from the configuration in one small exported check plus three call sites:conf_cnid_scheme_in_use()(netatalk_conf.c): scans the loaded volume list, then the[Homes]section — homes volumes are instantiated per-user at login, so outside AFP sessions they never appear in the master's volume list; the section's scheme is resolved exactly ascreatvol()resolves it (section → preset → global → compiled default). It lives in netatalk_conf.c because those resolution helpers are static there.cnid_metadwhen no volume resolves to dbd.cnid_metadasNETATALK_SRV_ERROR, which equalsNETATALK_SRV_NEEDED— without a guard the timer would resurrect a daemon a reload just made unnecessary; the guard demotes instead. This one check makes stop transitions converge through the existing machinery with no new state.Robustness: if the master's volume load fails, an empty list is indistinguishable from "no volumes", so a
volumes_loadedflag makes every decision fail conservative — the daemon runs, exactly as today. Builds without the dbd backend are unchanged: the pid sentinel can never become needed there (the SIGHUP re-evaluation is compile-gated), so the master never attempts to start a binary that isn't installed.Reload semantics
Start/stop on SIGHUP applies to added or removed volumes. Changing
cnid schemeon an existing volume takes effect at restart only —creatvol()does not change options for volumes once loaded, which is netatalk's long-standing reload behaviour for all volume options; this PR matches it rather than changing it.Testing
All cases run against live containers built from this branch.
cnid scheme = sqlitevolumescnid_metadcnid scheme = dbdvolumes[Homes], globalcnid scheme = dbdkill -9a neededcnid_metadmeson test-Dwith-cnid-backends=sqlite,mysql)cnid_metadnot installedcnid scheme = dbdcnid_open()has no backend fallback (verified)-Dwith-cnid-backends=dbd)sqlite(unavailable backend)(The dbd-only build cannot run the unit suite:
with-testsuses the sqlite backend because the cnid_metad / cnid_dbd daemon combo needs root privileges to run, so dbd was deliberately left out of the afpd tests.)