Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Slow Downloads stuck on 0kb size / MetaData Not being detected as slow. #119

Closed
wants to merge 12 commits into from

Conversation

slkiny
Copy link

@slkiny slkiny commented Jun 10, 2024

image

Currently these use cases fail to be detected. For torrents which are not very popular you may fail to even download the magnet file.

image

Instead it was detected as a big download that might be moving soon... This will fix the issue, and ensure that something is downloaded.

Example JSON of a Magnet File that is not downloaded.

[INFO]: {'seriesId': 90, 'episodeId': 8097, 'seasonNumber': 4, 'languages': [], 'quality': {'quality': {'id': 5, 'name': 'WEBDL-720p', 'source': 'web', 'resolution': 720}, 'revision': {'version': 1, 'real': 0, 'isRepack': False}}, 'customFormats': [], 'customFormatScore': 0, 'size': 0, 'title': 'fc2e9e29471ed570771be8dfa157c512fad6e6eb', 'sizeleft': 0, 'added': '2024-06-10T00:16:14Z', 'status': 'downloading', 'trackedDownloadStatus': 'ok', 'trackedDownloadState': 'downloading', 'statusMessages': [], 'downloadId': 'FC2E9E29471ED570771BE8DFA157C512FAD6E6EB', 'protocol': 'torrent', 'downloadClient': 'qbit', 'downloadClientHasPostImportCategory': False, 'indexer': 'TorrentDownload (Prowlarr)', 'episodeHasFile': False, 'id': 333611039}

image

Example logs when it is working.

@slkiny slkiny changed the title Fix stuck on meta data Fix Downloads stuck on MetaData Not being detected as slow. Jun 10, 2024
@ManiMatter
Copy link
Owner

ManiMatter commented Jun 10, 2024

Hi @slkiny

welcome and thank you for your PR.

I am not sure I understand what is happening here. On the picture below, the downloads are in "Downloading metatada" stage. On the json that you posted, they are in 'status': 'downloading', 'trackedDownloadStatus': 'ok', 'trackedDownloadState': 'downloading', 'statusMessages': []. So no mentioning of metdata.

image

I think I would not want to remove these metadata torrents in the slow-check. There's a specific script for that, and I would suggest that we amend that one if it fails to capture metadata items.

In the file "remove_metadata_missing.py", you see that this condition is there to capture items that fail to grab metadata:
if queueItem['status'] == 'queued' and queueItem['errorMessage'] == 'qBittorrent is downloading metadata':. However, that status is "queued", not downloading.

I'd like to understand why your queue-json does not say anything about metadata. Could you post the properties of the torrent that you see in qbit? Also, for how long are they in "downloading" state as opposed to "queued"?

Also, to make sure these metadata torrents (in downloading state) are not coming up in the slow check, I think what we can easily do is to change the below to if queueItem['status'] == 'downloading' and queueItem['size'] :

if queueItem['status'] == 'downloading':

@slkiny
Copy link
Author

slkiny commented Jun 15, 2024

Thanks for the feedback, yeah this seems like an edge case either the metadata missing or slow could detect. Maybe both..?

tldr; I think overall its tricky since its a download failing... Overall i think the root cause is more about detecting a torrent either being stuck (slow) downloading due to 0kb max size (bad metadata). Which to me should may be caught in both the slow and the metadata system..

I don't have a personal preference, but seems that you'd prefer it to be in the metadata handler.

I saw the metadata code was more about queued or errors. While this metadata seems to be the system thinks it can download it but cannot finish downloading it so I made the change in the slow download section.

Regardless of the final placement, another phrasing of this issue could be

"To Detect (slow) downloads that are not progressing 0kb; due to possible (metadata) data issues and have a size of 0 bytes for over X minutes, and remove those"

With that insight in mind is why I picked to put it in slow download, but i can see about moving it if you feel strongly about it.

I could imagine maybe a malfored metadata might also equal out to 0 byte size as well, so I am curious which was the root cause. But detecting 0 bytes/no size in general seems like a good thing to check for in many places.. since some people might not even enabled metacheck but might have slow check on hmm....

As a side note, i wanted to handle this in specific log message to the user. Since it helps to diagnose that the issue was due to metadata or torrent failing to download.

@slkiny
Copy link
Author

slkiny commented Jun 15, 2024

If you agree with the above statement we can modify the statement of the logs to be more about, a download with 0 size was detected. This looks malformed and if it doesn't progress or get a size > 0 soon we will terminate it etc..

changed wording.


  else:
      # MetaData is too slow to download/is stuck
      logger.info('>>> Detected %s download that is 0kb is slow or stuck. (Failed Metadata or Torrent) Adding to queue.',queueItem['title'])    
      affectedItems.append(queueItem)
      continue

By the way thanks for this project, I made my own prior. And found this to be a lot better :) I've had alot of oddities with qbit, which is why i prefer defense in depth so just check everything everywhere 😆

@slkiny slkiny changed the title Fix Downloads stuck on MetaData Not being detected as slow. Remove Slow Downloads stuck on 0kb size / MetaData Not being detected as slow. Jun 15, 2024
@ManiMatter
Copy link
Owner

Hi

thanks for your thoughts.

I am still unsure that I understand why this scenario even occurs. On the qbit screenshot that you posted the status is ‚Downloading metadata‘, but the json that you posted does not carry the status.

For me, slow check is for stuff in progress; metadata is a different gig, hence why I would put it over there. But rather to piecemeal together that the downloading of metadata is taking forever, it would be great if that status came through in the queue response. Maybe the sonarr team could add it and then we add that specific status to the metadata check?

@ManiMatter
Copy link
Owner

ManiMatter commented Jun 21, 2024

I still think we need to get to the bottom why your queue response is ‚downloading‘ (instead of ‚queued‘) and says nothing about metadata.

On the json that you posted, they are in 'status': 'downloading', 'trackedDownloadStatus': 'ok', 'trackedDownloadState': 'downloading', 'statusMessages': []. So no mentioning of metdata.

After all, there are other downloads that are marked like that when downloading metadata.

if queueItem['status'] == 'queued' and queueItem['errorMessage'] == 'qBittorrent is downloading metadata':

Maybe worthwhile asking the sonarr team?

What they probably need from you are the response of the /properties call in qbit for that specific torrent, so they see what the status etc is.

@slkiny
Copy link
Author

slkiny commented Jun 24, 2024

Yeah I tried looking into it more, but its still a curious bug; maybe related to how i tend to Force Download things possibly? I moved it to the metadata piece as suggested, but i dont see any harm in having it as a double check in general.

Also i added a shared func to check if the keys exist in a dict. A bit more readable.

@ManiMatter
Copy link
Owner

ManiMatter commented Jun 24, 2024

I think you should ask sonarr why the metadata state is not in errorMessage, and why the state is downloading, not queued. Because that is how it normally is; if they then might realize that they have not added the metadata state for downloading state, they add it and we use that.

Refer to this:
Sonarr/Sonarr#5940

Essentially what they achieved here is to differentiate between truly queued downloads, and those fetching metadata, by adding "statusMessages": ["qBittorrent is downloading metadata"].

It seems that you have hit a scenario where the status is not queued but downloading and the same differentiation is required. Maybe they can build on what they have already done for queued?

@ManiMatter
Copy link
Owner

@slkiny - are you still looking into this?

@slkiny
Copy link
Author

slkiny commented Jul 1, 2024

After the last refactor where i moved it into the Metadata checker, I stopped work then based on the previous conversation.

I think since it is now a generalized check to handle the edgecases that might show up in future or past builds of Sonarr/Radarr is good practice to detect when either an invalid torrent file gets added or similar when the size shows up as 0/0.

Another way of seeing it is, "Handle edgecase where file is malformed and has 0size" regardless of why it may have occurred.

But i appreciate what you did in getting "qBittorrent is downloading metadata" but to me the 0kb checker seems even easier; since we don't really care if its downloading metadata.. We just care if the size is invalid e.g. it should be over 0kb.

@ManiMatter
Copy link
Owner

In my view its not a problem of ‚malformed torrents‘ but that the radarr/sonarr app for some reason do not pass on the torrent status.

have you considered this suggestion?

I think you should ask sonarr why the metadata state is not in errorMessage, and why the state is downloading, not queued. Because that is how it normally is; if they then might realize that they have not added the metadata state for downloading state, they add it and we use that.

Refer to this: Sonarr/Sonarr#5940

Essentially what they achieved here is to differentiate between truly queued downloads, and those fetching metadata, by adding "statusMessages": ["qBittorrent is downloading metadata"].

It seems that you have hit a scenario where the status is not queued but downloading and the same differentiation is required. Maybe they can build on what they have already done for queued?

@slkiny
Copy link
Author

slkiny commented Jul 8, 2024

Hey, in general I think defense in depth is key. So if the upstream is stuck regardless of reason or status message that we should stop it/ remove it.

I'm happy with the current state and or the prior when we checked download status amount. Even if it gets fixed upstream it's always good to have double checks since then if there is a regression etc the program handles it regardless

@ManiMatter
Copy link
Owner

I see it differently; I would always suggest to first fix issues at source before implementing a workaround.

Particularly true with sonarr/radarr because they usually are quite responsive and helpful.

Please raise it with them as suggested above. Only if they don‘t move, I will consider merging this PR

@ManiMatter
Copy link
Owner

In my view its not a problem of ‚malformed torrents‘ but that the radarr/sonarr app for some reason do not pass on the torrent status.

have you considered this suggestion?

I think you should ask sonarr why the metadata state is not in errorMessage, and why the state is downloading, not queued. Because that is how it normally is; if they then might realize that they have not added the metadata state for downloading state, they add it and we use that.
Refer to this: Sonarr/Sonarr#5940
Essentially what they achieved here is to differentiate between truly queued downloads, and those fetching metadata, by adding "statusMessages": ["qBittorrent is downloading metadata"].
It seems that you have hit a scenario where the status is not queued but downloading and the same differentiation is required. Maybe they can build on what they have already done for queued?

Btw, just as another pointer. In Sonarr's code I see they do a specific handling for metadata when not forced:
https://github.com/Sonarr/Sonarr/blob/e97e5bfe8f3a7f01f7188f786eb7b51f1d8ef8b5/src/NzbDrone.Core/Download/Clients/QBittorrent/QBittorrent.cs#L283-L293

But I don't see any specific handling when it's forced. I'm not sure if this is of relevance for your problem, but maybe an extra input if you plan to you raise it with them.
https://github.com/Sonarr/Sonarr/blob/e97e5bfe8f3a7f01f7188f786eb7b51f1d8ef8b5/src/NzbDrone.Core/Download/Clients/QBittorrent/QBittorrent.cs#L298

If you don't plan to raise it with them, please close this issue.

@ManiMatter
Copy link
Owner

ManiMatter commented Jul 24, 2024

Sonarr/Sonarr#7001

@ManiMatter
Copy link
Owner

Considered fixed at source. Closing.

@ManiMatter ManiMatter closed this Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants