501187 - Memcached Issues

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Mike Lee]

With memcached on, I get the following (these are model issues):

User page:
- Tags do not update when a new tag is added via AJAX. When the addon
is reloaded, all new tags appear.
- When a tag is deleted and re-added via AJAX, it does not throw an
error like it used to.
- When a tag is deleted, it refreshes the tag list properly and all
added tags appear.

Developer tools page:
- Tags do not update when a new tag is added via AJAX. When the page
is reloaded, all new tags appear.

Admin tools page:
- No issues with memcached

Comment 1

[Mike Lee]

Notes from Yem:

Need to follow up with Patrick on this issue:
- When a tag is deleted and re-added via AJAX, it does not throw an
error like it used to.

Work still in progress.

Comment 2

[Yem Huynh]

got clarification from Patrick:

==========================
Yes, this wasn't very clear :)

Bad thing / unexpected : Tags do not update when a new tag is added via AJAX. When the addon is reloaded, all new tags appear.

Good thing:  When a tag is deleted and re-added via AJAX, it does not throw an error like it used to.

Good thing:  When a tag is deleted, it refreshes the tag list properly and all added tags appear.

Bad thing: Tags do not update when a new tag is added via AJAX. 
================================================================
So i will investigate into the ones marked 'Bad thing'.

Comment 3

[Yem Huynh]

Attachment: bug fix

tag list is now updated when adding tags (both as a dev and user ) due to fix on bug 501151 .  However what i found was that after a tag is added, refreshing the page makes the tag disappear.  I also noticed this happening on previews.  Attached is a fix but I'm not sure if it is the right thing to do.  However i do see this in other controllers where the beforeFilter() does something like this:

        // disable query caching so devcp changes are visible immediately
        foreach ($this->uses as $_model) {
            $this->$_model->caching = false;
        }

I think when adding the tag and the page is refreshed, the Addon is still cached, even though _add_tag() in tags_controller.php is calling markListForFlush()

Comment 4

[Wil Clouser [:clouserw]]

Attachment: use cache

Comment 5

[Wil Clouser [:clouserw]]

Comment on attachment 386427 [details] [diff] [review]
bug fix

Removing caching on our main add-on display page sounds like a recipe for disaster

Comment 6

[Wil Clouser [:clouserw]]

My patch uses the cache as intended but breaks inconsistently.  We're investigating that in bug 502144.  I'm reopening this as it's not fixed until a patch lands but there is no use working on this more until that bug is fixed.

Comment 7

[Jeff Balogh (:jbalogh)]

Comment on attachment 386653 [details] [diff] [review]
use cache

Works for me now that we fixed cake, but it would be nice to have a comment about what's going on here:

>+        if (empty($addon_data)
>+            || $addon_data['Addon']['inactive'] == 1
>+            || !in_array($addon_data['Addon']['addontype_id'], array(ADDON_EXTENSION, ADDON_THEME, ADDON_DICT, ADDON_SEARCH, ADDON_LPAPP, ADDON_PLUGIN))
>+            || !in_array($addon_data['Addon']['status'], $valid_status)) {

Like /* Make sure we're showing good add-ons */
and /* here's why I didn't do this in SQL */

Comment 8

[Wil Clouser [:clouserw]]

Thanks, I committed close to the same thing in r29331.  I'm pulling the contrib_details as well as array_unique()ing the $fields before we pass them off.