I think it's reasonably safe to assume that the search system can handle slightly stale data, since it does already. Attached patch converts most search-related queries to use slave servers if available.

And oh yeah, fixes lots of stray whitespace. Fix your IDEs, people!

CommentFileSizeAuthor
#9 search_slave.patch7.11 KBCrell
#2 search_slave.patch7.33 KBCrell
search_slave.patch6.73 KBCrell
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs work » Needs review
FileSize
search_slave.patch7.33 KB

Oopsies, missed a comma in one spot.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

Looks good. A site that really wants to index immediately after content submission can alter these queries.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Reviewed & tested by the community » Needs review

Actually, that would not be possible without a tag. Should we add a tag to these slave queries so they can be altered? Or automatically run these targetted queries through sql_alter()?

Crell’s picture

Crell CreditAttribution: Crell commented

hook_query_alter() cannot move a query from one connection object to another. That's bound at creation time. Remember, there's no requirement that two targets even be the same database driver, so it could be an entirely different class for the query object.

Remember that the slave server is disabled for the user that called a node_save() or comment_save() for a few minutes, so an instant-reindex WOULD appear to be instant for them, just not anyone else. For anyone else, they'll get it as soon as the slave catches up.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

Makes good sense.

Dries’s picture

Dries CreditAttribution: Dries commented 
+++ includes/database/select.inc	2009-10-27 03:27:18 +0000
@@ -1186,6 +1186,9 @@ class SelectQuery extends Query implemen
+    if (!is_string($alias)) {
+      debug($alias);
+    }

This looks like unwanted debug code, not?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Needs work
Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs work » Reviewed & tested by the community
FileSize
search_slave.patch7.11 KB

Unwanted? Don't be so rude to it. I may not want it, and you may not want it, but I'm sure someone wants that debugging code. Wouldn't you feel bad if someone called you unwanted? Think of what that does to its self-esteem? *sniff*

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

@Crell: You are a colossal dork. :)

My only hesitation in committing this is that the UX team really very badly wants #504012: Index content when created to get in, and it sounded like this patch would interfere with that. I talked to Crell on IRC about this though, and he pointed out that there would be two ways to address this should that other patch make it in:

<Crell> 1) It only indexes immediately if you're not doing Master/slave replication.  If you are, then the index will be up to date not immediately but as soon as the slaves catch up.  (Anywhere from immediate to a few minutes.)
<Crell> 2) Use the "temporarily disable slave server for this one user" feature we added a while back, so that one user will only use the master server for a few minutes.  So they'll see it in search results immediately but everyone else waits for the slave to catch up.

The fact that there's a workaround for this works for me, and we can cross that bridge when/if we get to it. It's probably arguable that high-performance sites using M/S replication don't want their content auto-indexed anyway.

Committed to HEAD.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.