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

Migrate dynamic collection updater to Web Service mode #64

Open
jesus2099 opened this issue Apr 27, 2015 · 15 comments
Open

Migrate dynamic collection updater to Web Service mode #64

jesus2099 opened this issue Apr 27, 2015 · 15 comments
Assignees
Labels
blocking maintainability MBS PR. Less features, code, libs. More comments. Ease maintenance and free up time. mb_COLLECTION-HIGHLIGHTER ninja server change
Milestone

Comments

@jesus2099
Copy link
Owner

jesus2099 commented Apr 27, 2015

  • more stability (less –almost not– not impacted by further MBS changes)
  • possible to be called outside the main release page (I will be able to revert e560d57 limiting)
@jesus2099 jesus2099 changed the title Migrate dynamic collection highlighter to Web Service mode Migrate dynamic collection updater to Web Service mode Apr 27, 2015
jesus2099 added a commit that referenced this issue Apr 27, 2015
…d in release sup‐pages

☞ removing dynamic collection updater from release sub‐pages until it uses Web Service (#64)
@jesus2099
Copy link
Owner Author

This ticket would also fix dynamic updater on 10+ medium releases (#133).

@jesus2099 jesus2099 added this to the 2015 milestone Dec 7, 2015
@jesus2099 jesus2099 modified the milestones: 2015, 2016 Dec 25, 2015
@jesus2099 jesus2099 modified the milestones: 2016 (late), 2017 (Ⅵ) Jun 28, 2016
@jesus2099 jesus2099 modified the milestones: 2017, 2018, 2019, 2019* Feb 10, 2017
@jesus2099 jesus2099 modified the milestones: 2021, 2022 Aug 21, 2021
@jesus2099 jesus2099 pinned this issue Aug 23, 2021
jesus2099 added a commit that referenced this issue Aug 23, 2021
Blocked by:
Release lookup web service recording-work relationships limit
https://tickets.metabrainz.org/browse/MBS-11905
@jesus2099 jesus2099 added the maintainability MBS PR. Less features, code, libs. More comments. Ease maintenance and free up time. label Aug 23, 2021
@jesus2099
Copy link
Owner Author

Work in progress blocked by MBS-11905:

https://github.com/jesus2099/konami-command/commits/ws-dynamic-updater_issue-64
Just one commit at the moment 1f99222

@jesus2099
Copy link
Owner Author

https://tickets.metabrainz.org/browse/MBS-11905?focusedCommentId=60125&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-60125

Nicolás Tamargo
Added 2021-08-23 19:20

The limit is 500 recordings, IIRC, and I don't think there's currently a way around it.

@jesus2099
Copy link
Owner Author

jesus2099 commented Aug 23, 2021

https://github.com/metabrainz/musicbrainz-server/blob/28916986f5f69a3fa9d081a770531f5f6983bc08/lib/DBDefs/Default.pm#L463-L466

# On release browse endpoints in the webservice, we limit the number of
# releases returned such that the total number of tracks doesn't exceed this
# number.
sub WS_TRACK_LIMIT { 500 }

@jesus2099
Copy link
Owner Author

jesus2099 commented Aug 23, 2021

https://github.com/metabrainz/musicbrainz-server/blob/f7d1d109e51b9c0313de3bdd64c1ce0f543e73f1/admin/DumpJSON#L41-L44

=head1 SYNOPSIS
[…]

Releases with more than 500 recordings ignore 'recording-level-rels', and
since those recordings aren't dumped separately, their relationships will
only be included in the target entities' output. As of 2017-04, this affects
~200 releases.

[…]
=cut

@jesus2099
Copy link
Owner Author

https://github.com/metabrainz/musicbrainz-server/blob/f7d1d109e51b9c0313de3bdd64c1ce0f543e73f1/lib/MusicBrainz/Server/Controller/WS/2/Release.pm#L145-L150

        # The maximum number of recordings to try to get relationships for, if
        # inc=recording-level-rels is specified. Certain releases with an enourmous
        # number of recordings (audiobooks) will almost always timeout, driving up
        # server load average for nothing. Ideally we can remove this limit as soon as
        # we resolve the performance issues we have.
        my $max_recording_relationships = 500;

@jesus2099
Copy link
Owner Author

metabrainz/musicbrainz-server@cff9928

Limit WS release lists to 500 tracks

This limits the results of release browse queries, including those from
the discid endpoint, to contain no more than 500 tracks (however, it
always returns at least one release, which may have more than 500
tracks on it).

With this change, certain browse queries with
inc=recordings+artist-credits specified can actually have a chance to
complete and not time out (which is more helpful to the client, since
they'll actually get information back, and more helpful to the server,
since it won't increase server load and hog worker processes for no end
purpose).

@jesus2099
Copy link
Owner Author

jesus2099 commented Aug 23, 2021

https://github.com/metabrainz/musicbrainz-server/blob/f7d1d109e51b9c0313de3bdd64c1ce0f543e73f1/lib/MusicBrainz/Server/ControllerBase/WS/2.pm#L319-L344

=head2 limit_releases_by_tracks

Truncates a list of releases such that the entire list doesn't contain more
than C<DBDefs::WS_TRACK_LIMIT> tracks in total (but returns at least one
release). The idea is to limit browse queries that contain an excessive number
of tracks when C<inc=recordings> is specified.

Note: This mutates the passed-in array reference C<$releases>.

=cut


sub limit_releases_by_tracks {
    my ($self, $c, $releases) = @_;


    my $track_count = 0;
    my $release_count = 0;


    for my $release (@{$releases}) {
        $c->model('Medium')->load_for_releases($release);
        $track_count += (sum map { $_->track_count } $release->all_mediums) // 0;
        last if $track_count > DBDefs->WS_TRACK_LIMIT && $release_count > 0;
        $release_count++;
    }


    @$releases = @$releases[0 .. ($release_count - 1)];
}

@jesus2099
Copy link
Owner Author

jesus2099 commented Sep 10, 2021

One good thing is that there is a way to detect that I am on a release that has skipped recording-level-rels, and not on a release that simply has no recording level relationships:

relations in release.media[m].tracks[t].recording.relations

In fact it is very good and logical.
When it decides to not send relationships, I think it returns same thing it would return when not asked for recording-level-rels.

@jesus2099
Copy link
Owner Author

jesus2099 commented Dec 19, 2021

Oh no, I lost my work in progress file, because of my recent official version (7854273) that triggered auto-update.
I should have saved my work in progress in git.
🤦

@jesus2099
Copy link
Owner Author

So MBS-11905 is not really blocking this.
I know recording.relations is absent (instead of empty array) when I got no recording-level-rels because of big release.

@jesus2099 jesus2099 removed the blocked label Dec 27, 2021
jesus2099 added a commit that referenced this issue Dec 31, 2021
Trying to understand the script before #64 #133 #174 changes
@jesus2099 jesus2099 self-assigned this Dec 31, 2021
jesus2099 added a commit that referenced this issue Jan 3, 2022
Still lacks new:

- loadMissingRecordingWorks
- Dynamic Updater #64 #133
@jesus2099
Copy link
Owner Author

jesus2099 commented Jan 10, 2022

Browsing web service JSON responses is also a lot of fine-tuning! so I will try my utmost to reuse the existing functions from initial loading #174, rather than keeping a specific dynamic loader loading.

jesus2099 added a commit that referenced this issue Jan 23, 2022
Dynamic loader action remove, still in web page mode
@jesus2099
Copy link
Owner Author

Dynamic remove not being converted to new mode, included release groups are not removed with dynamic remove (OFF or remove from collection).

@jesus2099 jesus2099 unpinned this issue Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking maintainability MBS PR. Less features, code, libs. More comments. Ease maintenance and free up time. mb_COLLECTION-HIGHLIGHTER ninja server change
Projects
None yet
Development

No branches or pull requests

1 participant