If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[ForumUX] Use newsearch to filter forum topics by poster (forum search too slow)

VERIFIED FIXED in 1.5

Status

support.mozilla.org
Forum
P3
normal
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: Cww, Assigned: paulc)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: sumo_only, URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

8 years ago
From: bug 472310 comment 16

-------

I recently noticed that bug 472310 still occurs if I use poster=cor-el (_me) or
other posters like poster=the-edmeister who post a lot.
Subsequent pages still show posts that belong on page one (comments_offset=0).
I had wondered for a while why I missed so many answers to threads where I
posted in.

Is this a new bug or is this bug not fixed for the poster Filter?

To speed up things:
[1]
https://support.mozilla.com/tiki-view_forum.php?forumId=1&comments_threshold=0&thread_sort_mode=lastPost_desc&comments_per_page=50&time_control=1209600&poster=cor-el&comments_offset=0
[2]
https://support.mozilla.com/tiki-view_forum.php?forumId=1&comments_threshold=0&thread_sort_mode=lastPost_desc&comments_per_page=50&time_control=1209600&poster=cor-el&comments_offset=50
[3]
https://support.mozilla.com/tiki-view_forum.php?forumId=1&comments_threshold=0&thread_sort_mode=lastPost_desc&comments_per_page=50&time_control=1209600&poster=cor-el&comments_offset=100
[4]
https://support.mozilla.com/tiki-view_forum.php?forumId=1&comments_threshold=0&thread_sort_mode=lastPost_desc&comments_per_page=50&time_control=1209600&poster=cor-el&comments_offset=150
[5]
https://support.mozilla.com/tiki-view_forum.php?forumId=1&comments_threshold=0&thread_sort_mode=lastPost_desc&comments_per_page=50&time_control=1209600&poster=cor-el&comments_offset=200
[6]
https://support.mozilla.com/tiki-view_forum.php?forumId=1&comments_threshold=0&thread_sort_mode=lastPost_desc&comments_per_page=50&time_control=1209600&poster=cor-el&comments_offset=250
[7]
https://support.mozilla.com/tiki-view_forum.php?forumId=1&comments_threshold=0&thread_sort_mode=lastPost_desc&comments_per_page=50&time_control=1209600&poster=cor-el&comments_offset=300
[8]
https://support.mozilla.com/tiki-view_forum.php?forumId=1&comments_threshold=0&thread_sort_mode=lastPost_desc&comments_per_page=50&time_control=1209600&poster=cor-el&comments_offset=350
[9]
https://support.mozilla.com/tiki-view_forum.php?forumId=1&comments_threshold=0&thread_sort_mode=lastPost_desc&comments_per_page=50&time_control=1209600&poster=cor-el&comments_offset=400
[10]
https://support.mozilla.com/tiki-view_forum.php?forumId=1&comments_threshold=0&thread_sort_mode=lastPost_desc&comments_per_page=50&time_control=1209600&poster=cor-el&comments_offset=450
(Assignee)

Comment 1

8 years ago
I've looked at this more closely, and the sort by lastPost actually isn't being done right (i.e. it's being done after the threads were already selected).

I'm attaching to patches: one for PHP and a proposed SQL patch to add some indexes that should speed up query time.
Assignee: nobody → paul.craciunoiu
OS: Linux → All
Hardware: x86 → All
(Assignee)

Comment 2

8 years ago
Created attachment 386388 [details] [diff] [review]
SQL patch: adding indexes to 4 important columns in tiki_comments

Cheng, you should take a look at this. I don't think any of these indexes are already there.

I should mention that it will take a while to add all those indexes, plenty of time to get some coffee :) This is due to the huge number of forum threads we have.
Attachment #386388 - Flags: review?(smirkingsisyphus)
Attachment #386388 - Flags: review?(laura)
Attachment #386388 - Flags: review?(cwwmozilla)
(Reporter)

Comment 3

8 years ago
I'm always in favor of more indexes but there's already a key on userName and commentDate and the existing two indices containing parentId may be enough.  (And there's already a bug for getting an index on type: bug 468461 which we should just fix.)

Here are the existing indexes:
  PRIMARY KEY  (`threadId`),
  UNIQUE KEY `no_repeats` (`parentId`,`userName`,`title`,`commentDate`,`message_id`(40),`in_reply_to`(40)),
  KEY `title` (`title`),
  KEY `data` (`data`(255)),
  KEY `objectType` (`object`,`objectType`),
  KEY `commentDate` (`commentDate`),
  KEY `hits` (`hits`),
  KEY `THREADED` (`message_id`,`in_reply_to`,`parentId`),
  KEY `hash` (`hash`),
  KEY `userName` (`userName`),
  KEY `parent_and_obj` (`parentId`,`objectType`),
  FULLTEXT KEY `ft` (`title`,`data`)
(Assignee)

Comment 4

8 years ago
Created attachment 386393 [details] [diff] [review]
patch, v1

This patch changes the current behavior. Now we use only one (larger) query to get all the data we need. 

I'd actually like to suggest a few more indexes used in this query. With this and the index updates, we should get decent query time.
Attachment #386388 - Attachment is obsolete: true
Attachment #386388 - Flags: review?(smirkingsisyphus)
Attachment #386388 - Flags: review?(laura)
Attachment #386388 - Flags: review?(cwwmozilla)
(Assignee)

Comment 5

8 years ago
Created attachment 386394 [details] [diff] [review]
SQL patch: adding indexes to 6 important columns in tiki_comments

This takes care of basically all the columns in the WHERE clause...
Attachment #386394 - Flags: review?(smirkingsisyphus)
Attachment #386394 - Flags: review?(laura)
(Assignee)

Comment 6

8 years ago
Oops. Sorry, Cheng, I didn't see your comment :)
I guess we can scratch the patch, or just add the non-existing ones.
(Reporter)

Comment 7

8 years ago
Other than type, you don't need new indexes.  You already have object+objectType and parentId+objectType as indexes which means that as long as you're putting all those into the same query, you don't need to add additional indexes on the individual columns.  So just adding the type one should be sufficient.
(Assignee)

Updated

8 years ago
Attachment #386393 - Flags: review?(smirkingsisyphus)
Attachment #386393 - Flags: review?(laura)
(Assignee)

Updated

8 years ago
Attachment #386394 - Attachment is obsolete: true
Attachment #386394 - Flags: review?(smirkingsisyphus)
Attachment #386394 - Flags: review?(laura)
(Assignee)

Updated

8 years ago
Summary: Replies made to older threads don't get put on front page when filtering on poster → [forumUX] Replies made to older threads don't get put on front page when filtering on poster
(Assignee)

Updated

8 years ago
Summary: [forumUX] Replies made to older threads don't get put on front page when filtering on poster → [ForumUX] Replies made to older threads don't get put on front page when filtering on poster
(Assignee)

Updated

8 years ago
Target Milestone: --- → 1.4
(Assignee)

Updated

8 years ago
Blocks: 504751

Comment 8

8 years ago
Just FYI, remember that you should only add an index if you absolutely need it.  Every extra index slows down INSERT, UPDATE, and DELETE queries, which are important for the forums.

Comment 9

8 years ago
Comment on attachment 386393 [details] [diff] [review]
patch, v1

I'm a little nervous about this - I think the sort order is right but the reason it was rewritten last time was because the query took way too long to run.  Can you provide benchmark numbers before and after patch?
(Assignee)

Comment 10

8 years ago
It seems that the query adds about 2.5 seconds to the load time, which is pretty bad. For example I got:
Before patch: 0.183s
After patch: 2.617s
(the time for get_forum_topics only)
The tests I did before I posted this patch weren't as bad, I probably had the queries cached in mysql.

I'm not sure what we can do. The thread ids approach will provide unexpected results, though, and miss certain threads.

One idea is to disable this filtering by poster and allow searches for it using newsearch, which is fast. The problem is that it won't provide up-to-date information, but a day old.

Ideas?
(Reporter)

Updated

8 years ago
Depends on: 468461
I like search using newsearch.  I was also thinking we could update the indexes more often.
Comment on attachment 386393 [details] [diff] [review]
patch, v1

Obviously, given the benchmarks, this is a no go.
Attachment #386393 - Flags: review?(laura) → review-
(Assignee)

Comment 13

8 years ago
Making this about adding the feature to newsearch. I've already worked on adding filters to search so yay :)
Summary: [ForumUX] Replies made to older threads don't get put on front page when filtering on poster → [ForumUX] Use newsearch to filter forum topics by on poster (forum search too slow)
(Assignee)

Updated

8 years ago
Summary: [ForumUX] Use newsearch to filter forum topics by on poster (forum search too slow) → [ForumUX] Use newsearch to filter forum topics by poster (forum search too slow)

Updated

8 years ago
Severity: major → normal
Target Milestone: 1.4 → Future
(Assignee)

Comment 14

8 years ago
Comment on attachment 386393 [details] [diff] [review]
patch, v1

Just going through my r? list
Attachment #386393 - Flags: review?(smirkingsisyphus)
(Assignee)

Comment 15

8 years ago
Seems like a search bug. Adding to 1.5.
Target Milestone: Future → 1.5
Priority: -- → P3
(Assignee)

Comment 16

8 years ago
Fixed as part of bug 501880.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Do we need links to this somewhere before we can resolve it, or are we just telling people who need to know how to use the advanced search form?
Do we need to reopen for comment 17?  Not sure what I should verify, here.
(Assignee)

Comment 19

8 years ago
I think linking to own posts (bug 504751) should be enough, but Cheng and David should decide.
Okay, thanks; if we need a better way, we could revisit this in another bug.

Verified FIXED on https://support-stage.mozilla.org/search.php?q=&status=0&author=cor-el&created=0&created_date=&lastmodif=0&sortby=1&where=f&locale=en-US&advanced=1 and https://support-stage.mozilla.org/search.php?q=&status=0&author=stephendonner&created=0&created_date=&lastmodif=30&sortby=1&where=f&locale=en-US&advanced=1 (couple different queries).
Status: RESOLVED → VERIFIED

Updated

8 years ago
Whiteboard: sumo_only
You need to log in before you can comment on or make changes to this bug.