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

Comments

msonnabaum’s picture

Status: Active » Needs review
StatusFileSize
new3.52 KB

New patch against the current repo.

david.moore.ipg’s picture

I 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.

ezra-g’s picture

Title: Remove unnecessary distinct from views, add caching to most_active_users » Remove unnecessary distinct from views, add caching to Views blocks

msonnabaum 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).

mstef’s picture

Patch in #2 causes duplicates on the status streams.

Query/explain:


mysql> explain SELECT activity_log_messages.mid AS mid, activity_log_templates.tid AS activity_log_templates_tid, activity_log_messages.aids AS activity_log_messages_aids, activity_log_messages.created AS activity_log_messages_created, activity_log_messages.last_updated AS activity_log_messages_last_updated, activity_log_messages.stream_owner_id AS activity_log_messages_stream_owner_id, activity_log_messages.stream_owner_type AS activity_log_messages_stream_owner_type, activity_log_messages.viewer_id AS activity_log_messages_viewer_id, activity_log_messages.target_id AS activity_log_messages_target_id, activity_log_messages.target_type AS activity_log_messages_target_type, activity_log_messages.acting_uid AS activity_log_messages_acting_uid, activity_log_messages.cached AS activity_log_messages_cached, activity_log_templates.rule AS activity_log_templates_rule, activity_log_templates.action_label AS activity_log_templates_action_label, activity_log_templates.template AS activity_log_templates_template, activity_log_templates.group_template AS activity_log_templates_group_template, activity_log_templates.group_summary AS activity_log_templates_group_summary, activity_log_templates.collapse_method AS activity_log_templates_collapse_method, activity_log_templates.pid AS activity_log_templates_pid, activity_log_templates.eval_input AS activity_log_templates_eval_input, activity_log_templates.resources AS activity_log_templates_resources, activity_log_templates.cacheable AS activity_log_templates_cacheable, activity_log_templates.display_type AS activity_log_templates_display_type FROM activity_log_messages activity_log_messages  LEFT JOIN activity_log_templates activity_log_templates ON activity_log_messages.tid = activity_log_templates.tid WHERE (activity_log_templates.display_type = 'web') AND (activity_log_templates.pid NOT IN (SELECT pid FROM activity_log_disabled_types WHERE uid = 1)) AND ( viewer_id = 0 OR viewer_id = 1 OR (viewer_id < 0 AND viewer_id <> -1) ) ORDER BY activity_log_messages_last_updated DESC;
+----+--------------------+------------------------+--------+---------------+---------+---------+-----------------------------------+------+-----------------------------------------------------+
| id | select_type        | table                  | type   | possible_keys | key     | key_len | ref                               | rows | Extra                                               |
+----+--------------------+------------------------+--------+---------------+---------+---------+-----------------------------------+------+-----------------------------------------------------+
|  1 | PRIMARY            | activity_log_messages  | ALL    | tid,viewer_id | NULL    | NULL    | NULL                              |  390 | Using where; Using filesort                         |
|  1 | PRIMARY            | activity_log_templates | eq_ref | PRIMARY       | PRIMARY | 4       | commons.activity_log_messages.tid |    1 | Using where                                         |
|  2 | DEPENDENT SUBQUERY | NULL                   | NULL   | NULL          | NULL    | NULL    | NULL                              | NULL | Impossible WHERE noticed after reading const tables |
+----+--------------------+------------------------+--------+---------------+---------+---------+-----------------------------------+------+-----------------------------------------------------+
3 rows in set (0.03 sec)
ezra-g’s picture

Status: Needs review » Needs work

Can you describe the duplicates so we can look into fixing them ;) ?

ezra-g’s picture

Priority: Normal » Major

I've updated the issue summary with a list of remaining work for this issue.

mstef’s picture

Views using DISTINCT:

  • facebook_status_all
  • facebook_status_conversation
  • facebook_status_stream
  • fbss_ur_stream
  • facebook_status_tags
  • facebook_status_mentions
  • facebook_status_private_messages
  • blog_latest_comments
  • content_blog_page (uses *new/updated* flag)
  • group_block_blog_latest (uses *new/updated* flag)
  • group_block_blog_latest_comments
  • group_tab_blogs (uses *new/updated* flag)
  • content_discussion_page (uses *new/updated* flag)
  • discussion_latest_comments
  • group_block_disc_latest_comments
  • group_block_discussion_latest (uses *new/updated* flag)
  • group_tab_discussions (uses *new/updated* flag)
  • content_document_page (uses *new/updated* flag)
  • document_latest_comments
  • group_block_doc_latest_comments
  • group_block_document_latest (uses *new/updated* flag)
  • group_tab_documents (uses *new/updated* flag)
  • content_event_calendar
  • content_event_page (uses *new/updated* flag)
  • event_latest
  • event_latest_comments
  • event_my_upcoming_events
  • event_past_events
  • group_block_events_latest
  • group_block_events_past
  • group_block_events_upcoming
  • group_block_evt_latest_comments
  • group_tab_events (uses *new/updated* flag)
  • upcoming_events
  • homepage_group_showcase
  • homepage_recent_featured (uses *new/updated* flag)
  • content_poll_page (uses *new/updated* flag)
  • group_block_poll_latest_comments
  • group_block_polls_latest (uses *new/updated* flag)
  • group_tab_polls (uses *new/updated* flag)
  • poll_latest_comments
  • subgroups_main_listing
  • content_wiki_page (uses *new/updated* flag)
  • group_block_wiki_latest (uses *new/updated* flag)
  • group_block_wiki_latest_comments
  • group_tab_wikis (uses *new/updated* flag)
  • wiki_latest_comments
  • facebook_status_followed
  • group_block_latest_content (uses *new/updated* flag)
  • group_block_members
  • groups_featured_groups
  • groups_main_listing
  • groups_most_active
  • groups_my_groups
  • groups_my_groups_listing
  • groups_newest_groups
  • my_recent_items
  • my_unread_items
  • newest_users
  • notice_latest_comments
  • og_my_content (uses *new/updated* flag)
  • tag_cloud
  • user_directory
  • users_online
ezra-g’s picture

Let's focus on the blocks that display on most/all pages:

My groups, Your/My groups, Most active users, Recent content, Newest groups

ezra-g’s picture

So, 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?

mstef’s picture

I 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.

lightsurge’s picture

I 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

mstef’s picture

I was looking at that.. the commit isn't connected to views though. Not sure..

mstef’s picture

Interesting..

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".

?

lightsurge’s picture

I'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)?

mstef’s picture

Looks 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):


mysql> select m.mid, m.target_id, m.target_type, m.acting_uid, m.tid, t.display_type from activity_log_messages m left join activity_log_templates t on t.tid = m.tid where mid in (413, 411);
+-----+-----------+-------------+------------+-----+--------------+
| mid | target_id | target_type | acting_uid | tid | display_type |
+-----+-----------+-------------+------------+-----+--------------+
| 411 |        78 | node        |          1 |   4 | web          |
| 413 |        78 | node        |          1 |   6 | web          |
+-----+-----------+-------------+------------+-----+--------------+
2 rows in set (0.00 sec)
lightsurge’s picture

How can they be dupes if they're using different activity log templates (activity logs only allows one template per message, right?)?

mstef’s picture

Well, 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).

mstef’s picture

Status: Needs work » Needs review
StatusFileSize
new19.52 KB

Patch to remove distinct and add a 30-minute query/output cache to:
- groups_newest_groups
- most_active_users

mstef’s picture

For 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.

mstef’s picture

Updated 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.

ezra-g’s picture

I broke out the patch from #18 into its own issue and committed at http://drupal.org/node/1372364#comment-5370320.

giorgio79’s picture

Was this for 7x? This stuff looks fab! :) (No version was set before)

ezra-g’s picture

Version: » 6.x-2.x-dev
Status: Needs review » Active

This is a 6.x-2.x issue. Thanks for the complement!

ezra-g’s picture

Issue summary: View changes

Next steps for resolving this issue.