zwave-js / zwave-js-ui

Full featured Z-Wave Control Panel UI and MQTT gateway. Built using Nodejs, and Vue/Vuetify
https://zwave-js.github.io/zwave-js-ui
MIT License
980 stars 202 forks source link

[bug] Cache file gets truncated upon service shutdown #2085

Closed gzone156 closed 2 years ago

gzone156 commented 2 years ago

Version

Checklist:

Build/Run method

Zwavejs2Mqtt version: 6.1.1 Z-Wave JS version: 8.9.0-beta.3

Describe the bug

Installed on RaspberryPiOs Bullseye using manual method (git clone / npm install / npm build / npm start). Upon service restart and/or system reboot, sometimes the cache file gets truncated, incurring in a subsequent re-interview of the entire network upon restart, and loosing some of HomeAssistant already mapped devices/entities (weird).

To Reproduce

Stop zwavejs2mqtt process

Expected behavior

Cache file doesn't get truncated

Attached is the main application log, in silly level and a screenshot of the instant the cache file got truncated.

Run1

zwavejs2mqtt_2021-12-21.log

Addendum: on a second run, with driver log enabled in silly level, the last row of the driver log says correctly "2021-12-21T11:14:53.792Z DRIVER destroying driver instance...", with no further entries logged

robertsLando commented 2 years ago

@gzone156 Could you try to send a SIGINT instead of a SIGTERM?

gzone156 commented 2 years ago

@robertsLando issuing a SIGINT via proper systemd service configuration did the trick, either during a manual service stop or a system reboot.

I noticed that the cache file is overwritten during service termination, either if no changes are made (and I can guess it is the most frequent scenario... network already setup and no changes made in nodes inclusion / exclusion or configurations). Shouldn't the cache file be left untouched in that case?

In any way, thanks for the super-prompt response.

robertsLando commented 2 years ago

I noticed that the cache file is overwritten during service termination, either if no changes are made (and I can guess it is the most frequent scenario... network already setup and no changes made in nodes inclusion / exclusion or configurations). Shouldn't the cache file be left untouched in that case?

I think that is by design, BTW that cache file is handled by zwavejs so I left @AlCalzone answer to this

AlCalzone commented 2 years ago

The way the json cache is built, its not easy to detect changes atm. https://github.com/zwave-js/node-zwave-js/issues/3091 should solve this when done.

robertsLando commented 2 years ago

@gzone156 please close this issue if you think it's fixed

gzone156 commented 2 years ago

Thanks, it's all clear now.

About the handling of the SIGTERM vs SIGINT, do you think it is an open subject? Should this issue be used for a deeper analysis of that subject, or will it be more appropriate to open an issue on zwave-js project?

robertsLando commented 2 years ago

I actually catch both signals but maybe for some reason SIGTERM doesn't give the app the time to close gracefully: https://github.com/zwave-js/zwavejs2mqtt/blob/master/app.ts#L1220

AlCalzone commented 2 years ago

The driver (node-zwave-js) doesn't handle signals. When the application destroys the driver it must make sure to give it enough time to save everything to disk.

gzone156 commented 2 years ago

Ok, I'll stick using SIGINT and close this issue.

Thanks for the wonderful job you're doing with this project! 🙇🏼