It was brought to my attention that my correct implementation of indexing has several severe performance-flaws, or really a single one:

if ($response->getHttpStatus() == 200) {
  $this->solr->commit(TRUE, FALSE, FALSE);
  return $ret;
}

This will execute both a COMMIT and an OPTIMIZE on the server, each time some items are indexed. As I now know, OPTIMIZE is a rather expensive operation which should better be executed once a day, than for each indexing operation.
Also, commits should better use Solr's auto commit facility, as they, too, aren't that cheap to do. This would lead to better performance when subsequently indexing several smaller batches of items (like will now happen with #946624: Use a cron queue for indexing).

Patch coming soon.

CommentFileSizeAuthor
#18 1225926--send-commit-delayed-18.patch2.98 KBdrunken monkey
#16 1225926--autoCommit-maxDocs-16.patch328 bytesdrunken monkey
#12 1225926--commitWithin.patch2.66 KBdrunken monkey
#4 1225926--always-commit-deletions-4.patch389 bytesdrunken monkey
#2 search_solr.patch2.47 KBfago
#1 1225926--commit-and-optimize.patch2.5 KBdrunken monkey

Comments

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review
StatusFileSize
new 1225926--commit-and-optimize.patch2.5 KB

This should fix this, and even take the "index immediately" option of indexes into account (committing added documents immediately if the option is set).

fago’s picture

fago
Primary language German
Location Vienna
commented
StatusFileSize
new search_solr.patch2.47 KB

This patch totally broke indexing without the immediate option. Attached patch fixes the problem.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Fixed

Ah, you're right. Weird that didn't trigger when I tested this …
Anyways – committed, thanks!

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Fixed » Needs review
StatusFileSize
new 1225926--always-commit-deletions-4.patch389 bytes

For security reasons, I guess it would be better to always commit after deletions, right?
Patch attached.

Or should we only do that when "Retrieve item data from Solr" is activated?

atlea’s picture

atlea commented

Patch from #4 fixed issue from #1233426-9: Dirty and queued items don't get removed from the tracking table when deleted where I was getting Fatal error in views on an index with deleted items the first < 120 seconds.

(I could not apply it using git apply, though - had to use patch)

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Fixed

OK, committed. Thanks for reviewing!

atlea’s picture

atlea commented

Batch importing new nodes now throws an "exceeded limit of maxWarmingSearchers=2". This is after just a few nodes (perhaps 10-20).

WD node: Exception: "503" Status: Service Unavailable: Apache Tomcat/6.0.24
Error report HTTP Status 503 - Error opening new searcher. exceeded limit of maxWarmingSearchers=2, try again later. in SearchApiSolrConnection->_sendRawPost() (line 95 of
/home/*/sites/all/modules/contrib/search_api_solr/solr_connection.inc)
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Fixed » Active

Not good … :-/
Do you have the latest dev versions of this module and Search API? And do you have „Index immediately“ enabled for the node index? Did you update the schema.xml and solrconfig.xml files used by Solr and restart it?

atlea’s picture

atlea commented

Yes, latest dev's. I believe I also have the latest schema.xml and solrconfig.xml but will recheck that. I tried adjusting the warmingsearchers to 4 (2 is a low number?) and tried to tweek the atuocommit settings with no luck (restarted solr/tomcat).
Did not have the time to debug further, but I will try again this evening. In my research into this I did stumble upon a few suggestions to use commitWithin in place of autocommit. Might be worth looking into. http://wiki.apache.org/solr/UpdateXmlMessages#Optional_attributes_for_.2...

atlea’s picture

atlea commented

A few more specs:
- I have one index without index immediately and two with. The two with is for just a few node types, so not as many records.
- The node type I was adding using node_save is not indexed at all.
- The total index is now at 10219.

atlea’s picture

atlea commented

I manage to partly recreate this on a different server. Using two indexes, one with index immediately and bundle filter set to strip out most of the nodes, about 5000 nodes total.

I got this error a couple of times during cron, and the rawPost was deletes. "<*lots of indexes*>". Not sure if that matters.

From what I can understand this error occurs when commiting too fast with autocommit turned on. The recommended solutions seems to be to use commitWithin (set to e.g. 120 ms). ..or commit once after running multiple operations?

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
StatusFileSize
new 1225926--commitWithin.patch2.66 KB

Did this work before this patch was committed? Otherwise, please open another issue.

However, as the Solr docs suggest this, too, I've now tried to implement this method of auto-commiting. Please see if the attached patch solves your problem.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Active » Needs review
atlea’s picture

atlea commented

I did not see this behavior before this commit. From what I understand this error is caused by sending inserts/deletes too fast when using autoCommit. I'll try the patch tonight or tomorrow when I can find the time. Thanks for looking into this.

atlea’s picture

atlea commented

YES, this seems to have fixed the issue. :)

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
StatusFileSize
new 1225926--autoCommit-maxDocs-16.patch328 bytes

Hm, that's a bit weird. Studying the documentation a bit further, there really shouldn't be any difference, at least in the time-based committing. Does this really work reliably?

Could you please also test …
- Whether the attached patch to solrconfig.xml also solves your problem?
- Whether deactivating „Index items immediately“ during the bulk import solves the problem?

To me, the latter (i.e., the commits after every inserted item caused by the „Index items immediately“ setting) looks far more likely to cause this bug, after doing a bit of research (= bugging other that know more about Solr).

mh86’s picture

mh86 commented

subscribing

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
StatusFileSize
new 1225926--send-commit-delayed-18.patch2.98 KB

The attached patch would delay commits until the end of the request. That way, in bulk imports, not every item would trigger a commit, but only every page request during the batch (I hope there isn't just one import per request). This would probably be a good idea in any case, and therefore neatly fit into the original goal of the issue (see its title).

Maybe this solves the problem, too? Would sound more promising to me, personally.

Anonymous’s picture

Anonymous (not verified) commented

Looks good, I applied the last patch and saw a nice performance increase. Before the patch, indexing my 1500 custom entities took about 90 seconds; after the patch only 25 seconds for 2000 entities. My test wasnt scientific of course, but anyway it seems to be good.

I didn't have any errors/warnings.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

Wow, great! Thanks for testing!
Now let's see whether that maybe also fixes atlea's issue, and I'd be really happy!

atlea’s picture

atlea commented

Does you patch from #18 require the patch form #16? Downloaded a fresh dev and applied #18, updated solrconfig.xml, restarted solr/tomcat6 and now it's not indexing at all...

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

No, on the contrary, they are mutually exclusive. (It would be good to know for both if they'd fix the issue, though. But #18 is more important.)
Hm, what exactly goes wrong for you? It obviously works for both morningtime and me.

atlea’s picture

atlea commented

Ok, so I was able to re-index by first disabling and then enabling the index. This seems to do a better job of clearing the index than "drush sapi-c ". As "drush sapi-i" is no good anymore either, perhaps your drush implementation needs some TLC? :)

Indexing without errors now. Speed is on par with the commitWithin patch, witch also seemed to speed things up quite a bit compared to pre-autoCommit (that was not working for me).

I too am leaning more toward a controled commit after the job is done, than relying on some magic that you only have partial control over, - hence my comment about "or commit once after running multiple operations".

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented

Hm, I can't really see why the drush commands shouldn't work anymore – but then again, I don't really maintain those, as I'm not using drush. (And what's „TLC“?)

However, glad that you also could get this to work and see performance improvements!

Anyone else want to test/review/comment/object? Otherwise I'll commit it in the next days.

atlea’s picture

atlea commented

Drush: The clear button in the UI did not work either, I had to disable/enable to get it to work. So guess it works the same in the UI as in drush. Indexing is not working either, but as you explained in another issue this is caused by new cron queue logic. Oh, and TLC = Tender loving care. ;)

BUT; i noticed you now reccomend using the r60 library. I had missed that, and was still on r22 - that could perhaps explain why I was having these issues. With the new patch it has been running real nice for the last couple of days, though. Even using r22.

Atle Andersen
New Reach
http://newreach.no

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Fixed

BUT; i noticed you now reccomend using the r60 library. I had missed that, and was still on r22 - that could perhaps explain why I was having these issues. With the new patch it has been running real nice for the last couple of days, though. Even using r22.

That shouldn't do any harm – the module should work with both versions equally well, for now.

And well, since the patch seems to be such a success, I now committed it. Thanks for reviewing, you two!

Status: Fixed » Closed (fixed)

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