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

Optimize music sync for large libraries #387

Merged

Conversation

mcarlton00
Copy link
Member

Instead of pulling all resources from the server in a single api call, utilize the "Paging - Max Items" setting and pull them in batches. Default value is 15 items per batch. Loads them with a generator so we can start processing them locally while still loading results from the server.

Changes the sync order a bit because of the way the loading happens. Instead of processing all of artist A, then moving on to all of artist B, it now syncs all artists, followed by all albums, with all songs at the end.

Slightly slower sync speeds than the previous iteration, but should be much safer when large libraries are concerned, and still much faster than 0.5.8. Fixes #386 and should prevent another situation similar to #380 from happening.

@mcarlton00
Copy link
Member Author

@n-buck if you can, I'd appreciate a test with your giant library to see if this works properly. Be warned that it's going to take a bit of time. As a comparison, my 9300 song library takes just over 3 minutes, and it's largely dependent on local device speed.

@niawag
Copy link

niawag commented Sep 9, 2020

I've tested this pull for Kodi 18 version (on coreelec, latest stable version) and it's working great. Thanks a lot!

jellyfin_kodi/downloader.py Outdated Show resolved Hide resolved
jellyfin_kodi/full_sync.py Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Sep 9, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@oddstr13 oddstr13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless there's a good reason to keep the batch object (anything other than Items being used?), I'd prefer if the for item in batch['Items'] loop was moved a couple of functions up (into _get_items).

That's something for another pullrequest however.

@mcarlton00 mcarlton00 merged commit c92b4a4 into jellyfin:master Sep 10, 2020
@mcarlton00 mcarlton00 deleted the i-like-big-libraries-and-i-cannot-lie branch September 10, 2020 00:37
@n-buck
Copy link

n-buck commented Sep 10, 2020

@mcarlton00 I just tested it and it works flawlessly with my library.
Thank you for the great and fast work!

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.

Music Sync fails on extra large libraries
5 participants