Open rkoshak opened 1 year ago
wborn
commented
1 year ago Ideally I'd like to see OH actually restore the backup instead of using it in place (which would be another indication that the backup is being used when the file reappears).
That may not always work when there is insufficient space left on the device resulting in empty databases and empty backups causing users to lose all their data (https://github.com/openhab/openhab-core/issues/2267).
rkoshak
commented
1 year ago That may not always work when there is insufficient space left on the device resulting in empty databases and empty backups causing users to lose all their data (https://github.com/openhab/openhab-core/issues/2267).
Attempting to copy the backup to the jsondb folder should not destroy the backed up file. If there is no space available the worst that should happen is that the restore fails and the restored file remains empty (and errors logged out somewhere). Restoring shouldn't be done through a move. All that is in line with #2267.
And it there is insufficient space left, that's a problem that needs to be addressed anyway. I'd rather have an indication that's a problem through an error now rather than OH just switching to use a backup file without telling me, masking that there's a problem.
My big concern here is that OH just silently starts using the backup file in place. There's no warning and no indication it's doing so. And in the case where someone really does want to wipe out one of the JSONDB files for some reason (e.g. testing though in this case it was intended to force the recreation of the marketplace file to accommodate changes there which couldn't be automatically applied) that doesn't happen. OH just silently switches over to the most recent backup leaving the user scratching their head about where the settings are coming from. In the end, I had to delete both the file in jsondb and all the backups too to force the recreation of the file, after spending a good deal of my own and @J-N-K's time trying to figure out why the settings remained active even after deleting the file.
But I'd be happy here with an error or warning log statement from OH saying "hey! I can't find file X but there's a backup so I'm going to use that!". But still, ideally I'd really like OH to actually restore the file and only use files in the jsondb folder, never run off of the actual backups.
And one thing I'm not sure I tested was what happens if OH is running off that backup and needs to save. Does it save to a new file in jsondb or does it write to the backup file? If the latter, that's really bad behavior. Per #2267, once backed up, the backup files should not be touched.
wborn
commented
1 year ago But I'd be happy here with an error or warning log statement from OH saying "hey! I can't find file X but there's a backup so I'm going to use that!".
It already seems to log that files are corrupt and backups are used. :thinking:
I guess it could try to copy the backup to the normal file once it is succesfully read.
rkoshak
commented
1 year ago I never got that log statement. That's weird.
Oh, that log.info gets printed when the file is corrupted but this line gets logged when the file doesn't exist: https://github.com/openhab/openhab-core/blob/c17481c2acc1fd5c005db38a3ce872be71d11685/bundles/org.openhab.core.storage.json/src/main/java/org/openhab/core/storage/json/internal/JsonStorage.java#L129
which is debug so I wouldn't have seen that.
I guess it could try to copy the backup to the normal file once it is succesfully read.
That's what I would have expected.
The behavior of OH when a JSONDB file is deleted is unexpected and counter intuitive.
If one deletes one of the files in the jsondb folder OH will silently start to use the most recent backup. There is nothing in the logs to indicated it is doing so. The backup is not actually restored either. It's just used in place.
I'm not sure what the "correct" behavior should be but I'm pretty sure this is not it. There should at least be something in the logs to indicate the backup is being used. Ideally I'd like to see OH actually restore the backup instead of using it in place (which would be another indication that the backup is being used when the file reappears).
It would also be nice to have an approach to clear it out instead of restoring that is easier than manually editing the file to
[]. Maybe delete the file and touch to replace it with an empty file or an API endpoint, or karaf command.Note: the file in question was org.openhab.marketplace.json so doing something like deleting all the Things or Items wouldn't work in this case.