i discovered this after excluding the search tables from an sql dump file, so that the new db loaded from the copy had no entries in the search_total table initially.

i believe this is because the search indexing is being done in a subshell. search_update_totals() depends on a static array that gets populated during the indexing, so it only has values in the subshell.

i put the register_shutdown_function('search_update_totals'); call into the front of $eval and that worked OK for me. i used that rather than a direct call to deal with errors that might terminate the indexing. not sure if that's best, but that's how the search module does it as far as i can tell.

i think the other calls to search_update_totals() in _drush_core_search_index() are moot.

(btw, the 7.x loop looks a bit odd - isn't it just resetting $eval each time, so that you end up with the last value? i don't have a 7.x install yet so i couldn't test it.)

CommentFileSizeAuthor
#3 search.totals2.patch1.25 KBbrad.bulger
search.totals.patch729 bytesbrad.bulger

Comments

moshe weitzman’s picture

moshe weitzman commented
Status: Active » Fixed

committed slightly changed version

7.x looks ok to my eyes. it is resetting $eval every time through the loop and it is executing it every time as well.

brad.bulger’s picture

brad.bulger commented

isn't the idea of registering a shutdown function so that if the indexing aborts, totals will still get updated from whatever did get done? that's why i put it in the front of $eval.

i have a 7.x site for testing now, and it's doing what i thought. $eval is being completely reset for each module in the search_active_modules array - *before* it is run. this is the result (with some added warnings):

$ drush search index
Building the index may take a long time.
Do you really want to continue? (y/n): y
Remaining items to be indexed: 1                                            [ok]
setting eval to module_invoke(node, 'update_index');                   [warning]
setting eval to module_invoke(user, 'update_index');                   [warning]
adding shutdown to module_invoke(user,                                 [warning]
'update_index');register_shutdown_function('search_update_totals');
No indexing is being done - 1 items before, 1 now - break out of loop    [error]
The search index has been built.

update_index is never invoked on node. so it will just keep going around through the loop.

brad.bulger’s picture

brad.bulger commented
StatusFileSize
new search.totals2.patch1.25 KB

updated version of patch attached

greg.1.anderson’s picture

greg.1.anderson commented
Status: Fixed » Needs review

Changing the status back to 'needs review' as a service. (Patch looks good by inspection, but I have not tried it.)

moshe weitzman’s picture

moshe weitzman commented

Oops. I committed #3 but forgot to change status here.

greg.1.anderson’s picture

greg.1.anderson commented
Status: Needs review » Fixed

My free service wasn't very useful then, was it? ;) Next time I'll check cvs to see if there was already a commit before I go changing the status.

brad.bulger’s picture

brad.bulger commented

i'm seeing a related problem i'd like to get some opinions about - i'll open this as a new issue if that'd be better:

the realname module uses a variable_get() result when it returns search status. the variable is updated by its implementation of hook_update_index(). but since that happens in the subshell, the parent drush process always only sees the initial status value. so it always sees the same number of items remaining to be indexed.

i could try to flush the variables somehow but for one, i don't immediately see how to do that without breaking things, and for another, it's a pretty dubious move anyway.

i think this is really an issue with the realname module, and would be similarly with any other module doing the same thing. does that sound correct?

would it make sense to also add some kind of loop detection into _drush_core_search_index(), that if the number of items remaining to be indexed doesn't change, it aborts?

moshe weitzman’s picture

moshe weitzman commented

Yes, that check for progress during index sounds like a good idea ... This static variable issue is a real annoyance. Otherwise, the subshell is very very useful. I'm not sure we can call this a bug in realname, its just that it is not built in a way thats friendly to multiple process indexing.

Status: Fixed » Closed (fixed)

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