vergoh / vnstat

vnStat - a network traffic monitor for Linux and BSD
GNU General Public License v2.0
1.36k stars 120 forks source link

Rationale for vnstat -u removal #169

Closed mpollice closed 3 years ago

mpollice commented 3 years ago

This is more of a question. I'd like to know what was the rationale for removing the -u option starting in version 2.0. Is there a way to replicate this functionality without it?

vergoh commented 3 years ago

There are few reasons why it became necessary / beneficial to drop the -u option from the vnstat command.

1) Using the -u command usually suggested that a cron entry was being used for the database updates. Usually cron entries can only be configured to repeat once every minute and often the default configuration in many distributions was 5 minutes. With network connection becoming faster, both options started becoming too slow if the system was for some reason using 32-bit traffic counters for the interfaces. 100 Mbit is enough to become problematic at 5 minute update interval and 1 Gbit requires the interval to be less than 30 seconds in order to stay accurate. Dropping -u allows having the daemon handle the updates and it can also override user configuration if necessary when those don't make sense based on seen interface speeds. The daemon also has the benefit of having the ability to cache data so it doesn't need to write the data to the database every time an update is internally performed.

2) While the daemon was slowly getting migrated to be the default option in many distribution specific binary packages, some users still kept using vnstat -u from the command line and then ended up being confused with the end result. If the daemon was running then it would eventually overwrite the -u updated database content which cached data which could result in reversals of content from the user prespective. Forcing the daemon to be the only option removed the possibility for this sort of mismatch.

3) In the early versions, vnstat -u was always being executed as root from the cron entry and this didn't really cause much issues. At some point, some distributions opted to create a separate vnstat user for running vnstat -u and having ownership of the database files. However, by this time, it was far too easy to find invalid instructions with a Google search that would instruct executing something like sudo vnstat -u that would result in database files getting created as root and then blocking updates with the vnstat -u cron job running as vnstat user. Later versions had the executing user as a configuration option and the process would check and fix the file ownerships before switching user if started as root but that didn't really improve the situation much. Dropping -u support made executing those already invalid instructions no longer possible and stopped them causing further harm.

What is the use case where you'd still require -u to exist?

mpollice commented 3 years ago

Thank you for the elaborate response.

I wasn't aware of the issue described in 2). I'd have expected that vnstat -u would signal the update to vnstatd or that vnstatd would detect the modification. But one could also make vnstat warn about the demon running and abort.

As for the use cases, one would be running it in a config as described in 1) in a very low resource environment (think of an embedded device with low RAM). Usually very high interface speeds aren't an issue in this case. The other use case where I regularly need it is again embedded devices like openwrt routers (but not necessarily low RAM ones). Before a planned reboot or power outage I'd run vnstat -u, backup the db, reboot etc and later restore. WIthout this I would have lost some data (depending on the config).

vergoh commented 3 years ago

The vnstat and vnstatd don't have any API for communicating between each other directly. vnstat only reads the database regardless of vnstatd being running or not while vnstatd is tasked to write data to that database without knowing is anyone is reading it. That keeps the design simple and often simple results in also reliable. I'm not currently even sure what would be the best way to implement some interaction or discovery between these two processes that would be generic enough to work in all environments and systems that vnStat currently supports.

Regarding the memory usage, in vnStat versions 1.x the daemon keeps a copy of all interface databases always in memory which then gets updated and written to disk. On the other hand, versions 2.x keep only the unwritten data in memory without ever loading the full database. The write operations mainly only instruct the values in the database to be incremented. As a result, the version 2.x daemon may use less memory in reality then versions 1.x. That's however something I haven't exactly verified. Having the sqlite library included may results in gains from not keeping the full database in memory being lost.

From a low resource environment point of view, I'm also not sure if it's better to have a process constantly running in the background resulting in a known and predictable memory usage or starting processes on an interval that will result in spikes in the memory usage. Note that crond and the shell spawned to execute the vnstat -u command would also eat some memory.

The vnstatd process will flush it's cache to disk when stopped but of course this doesn't much help with direct power outages. The process however does listen to the SIGHUP signal (as documented also in the man page) which causes it to flush all data to disk when received without exiting. This is most likely the feature you are looking for.

mpollice commented 3 years ago

The most basic way to communicate could be a separate file that tells either program about the last written and daemon running status. Whether that is an elegant implementation is another matter. Querying the db would be the heavy handed approach. A custom signal may be another way I can think of. But I'm not requesting them to communicate. I would have just assumed without verifying that the case you described earlier where a running vnstatd would overwrite the manual update wouldn't happen.

Regarding the embedded systems case, now that I think of it, due to the switch to sqlite there may be cases where one would want to stick with 1.x anyway as sqlite adds to the binary size considerably. Unless sqlite would be used in other software as well this would be rather undesirable. Of course it depends a lot on the available resources and desired functionality.

The constant vs spike usage thing is certainly something to consider. I didn't really evaluate it indepth. I used such a cron-based setup in a minimalistic setup of an outdated router and it worked quite well. In openwrt cron would be running anyway for other reasons as well, as would the shell.

Thanks for the last pointer. So instead of vnstat -u to update the db to the latest state I'd send a SIGHUP and the rest would be the same (aside from the fact the db is now a single file).