Anthirian / script.service.janitor

Janitor is an addon for Kodi that cleans up any watched videos from your hard drive(s) based on various criteria
GNU General Public License v3.0
26 stars 14 forks source link

No episodes are cleaned when "Do not clean videos that have hard links" is enabled #87

Closed wutr closed 3 years ago

wutr commented 3 years ago

ls -la shows there is only one hard link to the episodes in question (as in, the pointer to the inode). There may have been more than one hard link to all involved files in the past, but when running Janitor there was only 1. When I disable "Do not clean videos that have hard links" the cleaning process works as expected. So why isn't it cleaning when that setting is enabled, even though there is only one hard link?

Anthirian commented 3 years ago

Could you please provide me with a debug log? Also did you try out the new Kodi 19 Support branch to see if the issue remains?

wutr commented 3 years ago

Here is the output with Janitor set to write debug messages when the problem occurred. After turning off "Do not clean videos that have hard links" there are no messages at all, but it functions as expected. The log doesn't seem overly helpful but it helped me in finding the part of the code where it checks for hard links, which led me to turn that off (even though I might need it).

I haven't tried the Kodi 19 branch (yet).

2021-02-17 20:03:57.725 T:1472185216  NOTICE: Janitor: Not cleaning nfs://192.168.0.101/volume1/media/series/episode1.mkv.
2021-02-17 20:03:59.749 T:1472185216  NOTICE: Janitor: Not cleaning nfs://192.168.0.101/volume1/media/series/episode2.mkv.
2021-02-17 20:04:01.773 T:1472185216  NOTICE: Janitor: Not cleaning nfs://192.168.0.101/volume1/media/series/episode3.mkv.
2021-02-17 20:04:03.794 T:1472185216  NOTICE: Janitor: Not cleaning nfs://192.168.0.101/volume1/media/series/episode4.mkv.
2021-02-17 20:04:05.819 T:1472185216  NOTICE: Janitor: Not cleaning nfs://192.168.0.101/volume1/media/series/episode5.mkv.
2021-02-17 20:04:07.851 T:1472185216  NOTICE: Janitor: Not cleaning nfs://192.168.0.101/volume1/media/series/episode6.mkv.
2021-02-17 20:04:09.881 T:1472185216  NOTICE: Janitor: Not cleaning nfs://192.168.0.101/volume1/media/series/episode7.mkv.
2021-02-17 20:04:15.221 T:1472185216 WARNING: CPythonInvoker(7, /storage/.kodi/addons/script.service.janitor/default.py): the python script "/storage/.kodi/addons/script.service.janitor/default.py" has left several classes in memory that we couldn't clean up. The classes include: N9XBMCAddon4xbmc7MonitorE,N9XBMCAddon7xbmcgui14DialogProgressE
2021-02-17 20:05:46.591 T:1936850352  NOTICE: NFS is idle. Closing the remaining connections.
Anthirian commented 3 years ago

Did you also enable debug logging in Janitor itself? I only see loglevel NOTICE here, which suggests that's not the case. Also the new version is still in development, but I did make some changes to the hard link related settings, so maybe you can try that out and create a new bug report? Be sure to make a back up of any important files, because things may very well break still.

wutr commented 3 years ago

Did you also enable debug logging in Janitor itself?

Debug logging was enabled in Janitor, but should I enable it elsewhere (in Kodi) too?

Anthirian commented 3 years ago

No that's not required. The setting in Janitor simply toggles the log level to be more verbose. Since I write a lot of messages to the log on the DEBUG log level, those are hidden unless you enable to setting in Janitor itself. I'm guessing this particular case just didn't write anything as DEBUG, but only NOTICE.

wutr commented 3 years ago

Alright, in that case I will have to try the Kodi 19 branch.

wutr commented 3 years ago

I've attempted to run the Kodi 19 branch of Janitor on my Kodi 18 install (Libreelec 9.2.6 on pi 3b) but it turns out it isn't compatible. I haven't tried 'hacking' it together, but looking at the code makes me think there are no major changes that would have an impact on the hardlink checking. So unless there is a change in the xbmcvfs module from 18 to 19, I don't think there will be any difference. I'd need to run the xbmcvfs.Stat.st_nlink function on any of the files to see what it returns, but it's not accessible from outside Kodi. So the next step is to modify the code of the 18 branch so that I can get some insight into the output of xbmcvfs.Stat.st_nlink. Oh and all the debug statements seem to not output anything in the logs. Please see the my next post for an update.~~

Kodi 18 branch:

def has_no_hard_links(self, filename):
        if get_setting(keep_hard_linked):
            debug(u"Making sure the number of hard links is exactly one.")
            is_hard_linked = all(i == 1 for i in map(xbmcvfs.Stat.st_nlink, map(xbmcvfs.Stat, self.unstack(filename))))
            debug(u"No hard links detected." if is_hard_linked else u"Hard links detected. Skipping.")
        else:
            debug(u"Not checking for hard links.")
            return True

def unstack(self, path):
        if path.startswith(u"stack://"):
            debug(u"Unstacking {0}.".format(path))
            return path.replace(u"stack://", u"").split(u" , ")
        else:
            debug(u"Unstacking {0} is not needed.".format(path))
            return [path]

Kodi 19 branch:

def is_hardlinked(filename):
    if get_value(keep_hard_linked):
        debug("Hard link checks are enabled.")
        return not all(i == 1 for i in map(xbmcvfs.Stat.st_nlink, map(xbmcvfs.Stat, split_stack(filename))))
    else:
        debug("Hard link checks are disabled.")
        return False

def split_stack(stacked_path):
    return [element.replace("stack://", "") for element in stacked_path.split(" , ")]
wutr commented 3 years ago

Update: from looking at the debug function I found that debug loggin does have to be enabled in Kodi itself. So I did that and here's the partial result, which should be sufficient to see that the hardlinking code correctly determines there are no hard links, but fails to properly translate that into the correct condition in order to remove the file:

In fact I just noticed I missed in my previous post that there is no return statement in the Kodi 18 branch when checking for hardlinks. I think that might be the problem?

def has_no_hard_links(self, filename):
        if get_setting(keep_hard_linked):
            debug(u"Making sure the number of hard links is exactly one.")
            is_hard_linked = all(i == 1 for i in map(xbmcvfs.Stat.st_nlink, map(xbmcvfs.Stat, self.unstack(filename))))
       ## this might be a fix:
            if not is_hard_linked:
                debug(u"No hard links detected")
                return True
            else:
                debug(u"Hard links detected. Skipping.")
                return False
       ## end of the fix
        else:
            debug(u"Not checking for hard links.")
            return True
2021-03-06 10:18:56.942 T:1799328640   DEBUG: Janitor: Found 8 videos that may need cleaning.
2021-03-06 10:18:56.942 T:1799328640   DEBUG: Janitor: Unstacking nfs://192.168.0.101/volume1/media/series/episode1.mkv is not needed.
2021-03-06 10:18:56.993 T:1799328640   DEBUG: Janitor: Making sure the number of hard links is exactly one.
2021-03-06 10:18:56.993 T:1799328640   DEBUG: Janitor: Unstacking nfs://192.168.0.101/volume1/media/series/episode1.mkv is not needed.
2021-03-06 10:18:57.000 T:1799328640   DEBUG: Janitor: No hard links detected.
2021-03-06 10:18:57.001 T:1799328640  NOTICE: Janitor: Not cleaning nfs://192.168.0.101/volume1/media/series/episode1.mkv.
2021-03-06 10:18:57.001 T:1799328640   DEBUG: Janitor: Progress percent is 12.5, amount is 8 and increment is 0.125
wutr commented 3 years ago

I've written the fix and will create a pull request soon.

Pull request done, I've tested the code and it correctly determines if a file has only one, or more than one hard links and returns True or False correctly. It now cleans (or doesn't clean) files as it should when the option to keep files with hard links is enabled!

Anthirian commented 3 years ago

I see, apparently when refactoring my code at some point I must have removed the default return True statement that supposed to be there. Thanks for the pull request, it looks good. However, since the new version I'm developing does these hard link checks in a different way, merging your pull request will cause issues merging the new branch further down the road. Furthermore, I only ever support the newest version of Kodi, because trying to maintain compatibility with one version is already quite difficult with my day job. All this means that the change you made will be of use for a very short amount of time (until Janitor is compatible with Kodi Matrix). I will therefore close this issue and corresponding pull request, but you're of course free to use these changes for your local install until the new version comes out. Anyone having the same issue can of course also make use of the changes described in your pull request if they dislike waiting for the new version to come out. Anyway, I do appreciate you taking the time to help solve the issue very much. Feel free to open another issue should the new version still not solve the issue you're having right now, and I will happily look into it again.

wutr commented 3 years ago

All good, I understand your reasoning and am glad there is a solution nonetheless.