Remove getAuthors call from getListAddons

RESOLVED FIXED

Status

--
critical
RESOLVED FIXED
11 years ago
3 years ago

People

(Reporter: clouserw, Assigned: wenzel)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
For performance reasons (and because it's superfluous?) we need to remove this call.  Instead of doing this call separately it should be a part of the HABTM relationship between authors and add-ons.
(Assignee)

Comment 1

11 years ago
Created attachment 325788 [details] [diff] [review]
removing getAuthors call, using HABTM association rule

there you go.
Attachment #325788 - Flags: review?(clouserw)
Part of the reason fligtar did it this way was that cake doesn't work well with metadata stored in a map table.  In this case addons_users stores the meta for listed/position... so it's not pointless, we should just find a better way I think.
(Reporter)

Comment 3

11 years ago
Comment on attachment 325788 [details] [diff] [review]
removing getAuthors call, using HABTM association rule

adding morgamic to the review list since he is set up to make sure this patch performs well
Attachment #325788 - Flags: review?(morgamic)
(Reporter)

Comment 4

11 years ago
Comment on attachment 325788 [details] [diff] [review]
removing getAuthors call, using HABTM association rule

Functionally, it works for me.  My r+ dependent on morgamic's perf testing.
Attachment #325788 - Flags: review?(clouserw) → review+
Comment on attachment 325788 [details] [diff] [review]
removing getAuthors call, using HABTM association rule

Could you add the appropriate indexes to addons_users as well?
Attachment #325788 - Flags: review?(morgamic) → review-
(Assignee)

Comment 6

11 years ago
(In reply to comment #4)
> (From update of attachment 325788 [details] [diff] [review])
> My r+ dependent on morgamic's perf testing.

Okay let's wait for that. Though I vote for checking this in like this anyway even if it doesn't "dramatically" improve system load, as it's cleaner than what we have now. (Unless of course my patch has a negative impact, which I doubt).
I saw 12k -> 5k for dbsize, so that looks like it has the desired effect.
(Assignee)

Comment 8

11 years ago
(In reply to comment #5)
> (From update of attachment 325788 [details] [diff] [review])
> Could you add the appropriate indexes to addons_users as well?

We already have an index over addon_id and user_id, and adding additional indexes won't make them being used by MySQL.

I tested and everything works fine for me as well - Thanks.
Comment on attachment 325788 [details] [diff] [review]
removing getAuthors call, using HABTM association rule

Nevermind, index won't help, +r=me
Attachment #325788 - Flags: review- → review+
(Assignee)

Comment 11

11 years ago
Thanks, everyone. r16058.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: push-needed
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Keywords: push-needed
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.