Remaining work in this issue:
- Test removal of DISTINCT
- Identify specific problems with removing DISTINCT (What is duplicated? Is there a solution to that duplication aside from adding DISTINCT to the view?)
- Identify which blocks can be easily cached, which blocks need more complex caching logic and some options for that logic.
Sorry to bundle this all in one patch, but this removes the "distinct" flag from four views (there are probably more) that were causing unnecessary DISTINCT queries when viewing the streams page.
Also added a one minute cache on most active users. The query for that view as it is can't use an index and will get pretty rough as the user table grows. Since the view isn't user-specific, we should at least try to cache it as a temporary solution. I'd like to see the cache time higher than a minute, but I figured I'd be on the conservative side here.
Also, the patch applies against the features repo here: http://drupal.org/sandbox/ezrag/1262136
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | drupal_commons-cache-and-remove-distinct-1272620-2.patch | 93.55 KB | mstef |
| #18 | drupal_commons-cache-and-remove-distinct-1272620.patch | 19.52 KB | mstef |
| #1 | commons_views_performance_fixes-1272620-1.patch | 3.52 KB | msonnabaum |
| views_performance_fixes.patch | 3.41 KB | msonnabaum |
Comments
Comment #1
msonnabaum commentedNew patch against the current repo.
Comment #2
david.moore.ipg commentedI wondered if this might have been a hack for when events are associated with multiple groups. When you create an event, you can select multiple groups to post in. I know this creates multiple notifications (if I am in three groups that the event is posted to I get three separate notifications). I wonder if removing the distinct in those views will cause crossposted events to appear multiple times in calendars and "my groups" type views.
Comment #3
ezra-g commentedmsonnabaum also points out that we have a lot of blocks (My groups, notices, most active users) that don't need to be updated frequently but are doing queries and using render resources on most or all page requests for authenticated users.
Let's cache those. As a future step (probably for another issue) we could potentially use http://drupal.org/project/cache_actions to cache them for longer and invalidate at specific times (eg user joins a group).
Comment #4
mstef commentedPatch in #2 causes duplicates on the status streams.
Query/explain:
Comment #5
ezra-g commentedCan you describe the duplicates so we can look into fixing them ;) ?
Comment #6
ezra-g commentedI've updated the issue summary with a list of remaining work for this issue.
Comment #7
mstef commentedViews using DISTINCT:
Comment #8
ezra-g commentedLet's focus on the blocks that display on most/all pages:
My groups, Your/My groups, Most active users, Recent content, Newest groups
Comment #9
ezra-g commentedSo, groups_newest_groups and most_active_users are two that we should be able to remove distinct from and cache aggressively without side-effects. Per #5, @mstef can you post a description of the duplicates you were getting with the patch from #1?
Comment #10
mstef commentedI saw no logical pattern for it. The only items that were repeating were "node add" messages, but not all of them were. I can't say for sure that those are the only messages that will duplicate, mainly because, for some reason, my streams were only showing "node add" and "group join" messages (there should be a lot more, right?). It doesn't seem like manually-entered statuses are duplicating.
Comment #11
lightsurge commentedI remember there was an issue where node add messages were getting duplicated even with distinct was enabled. Is it possible that the status items are actually duplicated?
#1306252: Duplication of messages about node creation in group for acting user
See also..
#1250820: The activity_log views should have a uniqueness filter
Comment #12
mstef commentedI was looking at that.. the commit isn't connected to views though. Not sure..
Comment #13
mstef commentedInteresting..
I took the query above, and ran it myself, analyzing exactly what was being sent back, and surprisingly, there are no duplicate entries (based on the message ID). So.. I added the message ID to the status stream view, and the "duplicates" have different message IDs. Perhaps they're being saved twice in the database - but if that's the case, why/how is DISTINCT removing the "dupes".
?
Comment #14
lightsurge commentedI'm getting confused between AIDs and MIDs here... Do activity log entries have a unique MID for a particular message, but a secondary key in the form of an AID attached to each occurrence of that same message which appears slightly differently (i.e. different visibilities etc)?
Comment #15
mstef commentedLooks like the issue is having two rules saving duplicates:
- Log activity when a node is created in a group
- Email: Log activity when a node is created in a group
--
Clearly, the first is used to generate email digests, so perhaps two messages should be stored. But the question is then, 1) Why are both showing on the stream (there's a filter that's supposed to prevent this) and 2) How could distinct be identifying dupes if the different messages?
The query on the dupes is showing that they're both web display though (not email):
Comment #16
lightsurge commentedHow can they be dupes if they're using different activity log templates (activity logs only allows one template per message, right?)?
Comment #17
mstef commentedWell, they're technically not dupes (see mid). And now, for some reason, turning distinct back on, the "dupes" are still showing.
I'm getting lost with this issue now. The rules are set to put one message for "web" and one for "email", but it looks like they're both going to "web" (as shown in the above query).
Comment #18
mstef commentedPatch to remove distinct and add a 30-minute query/output cache to:
- groups_newest_groups
- most_active_users
Comment #19
mstef commentedFor the activity stream dupes, I think we should keep the conversation @ #1306252: Duplication of messages about node creation in group for acting user or #1256642: Duplicate status updates. The views distinct doesn't seem to be an issue, so I was wrong for thinking that it fixed it by turning it on. Distinct can be removed from the view, and I think it already has.
Comment #20
mstef commentedUpdated patch:
- groups_newest_groups (added 30m cache / removed distinct)
- most_active_users (added 30m cache / removed distinct)
- groups_most_active (added 30m cache / removed distinct)
- groups_featured_groups (removed distinct)
- groups_my_groups (removed distinct)
- content_blog_page (removed distinct)
- content_discussion_page (removed distinct)
- content_document_page (removed distinct)
- content_event_calendar (removed distinct)
- content_event_page (removed distinct)
- content_poll_page (removed distinct)
- content_wiki_page (removed distinct)
- event_latest (removed distinct)
- event_my_upcoming_events (removed distinct)
- event_past_events (removed distinct)
- group_block_members (removed distinct)
- group_block_latest_content (removed distinct)
- group_block_events_upcoming (removed distinct)
- group_block_events_past (removed distinct)
- group_block_events_latest (removed distinct)
- group_block_document_latest (removed distinct)
- group_block_discussion_latest (removed distinct)
- group_block_blog_latest (removed distinct)
- group_block_wiki_latest (removed distinct)
- group_tab_wikis (removed distinct)
- group_tab_polls (removed distinct)
- group_tab_events (removed distinct)
- group_tab_documents (removed distinct)
- group_tab_discussions (removed distinct)
- group_tab_blogs (removed distinct)
- group_block_polls_latest (removed distinct)
- users_online (removed distinct)
- user_directory (removed distinct)
- upcoming_events (removed distinct)
- subgroups_main_listing (removed distinct)
- recent_notices (removed distinct)
- profile_user_groups (removed distinct)
- newest_users (removed distinct)
- my_unread_items (removed distinct)
- my_recent_items (removed distinct)
- homepage_recent_featured (removed distinct)
- homepage_group_showcase (removed distinct)
--
Notes:
- Comment views seemed to need distinct to avoid duplicates.
Comment #21
ezra-g commentedI broke out the patch from #18 into its own issue and committed at http://drupal.org/node/1372364#comment-5370320.
Comment #23
giorgio79 commentedWas this for 7x? This stuff looks fab! :) (No version was set before)
Comment #24
ezra-g commentedThis is a 6.x-2.x issue. Thanks for the complement!
Comment #24.0
ezra-g commentedNext steps for resolving this issue.