Error in apachesolr_index_get_entities_to_index() function

yes, looks like a correct fix.

I changed the function a little bit more so it becomes more consistent. Would you be so kind to verify? :-)

+++ b/apachesolr.index.incundefined
@@ -306,7 +306,7 @@ function _apachesolr_index_process_entity_get_document($entity, $entity_type) {
-  if (function_exists('drupal_get_path_alias')) {
+  if (function_exists('drupal_get_path_alias') && !empty($document->path)) {

Seems like I included a document->path change here. This is needed because not all the documents that are created by contrib modules provide the path in the document.

+++ b/apachesolr.index.incundefined
@@ -374,13 +374,13 @@ function apachesolr_index_get_entities_to_index($env_id, $entity_type, $limit) {
-          ->condition('entity_id', $id)
+          ->condition('entity_type', $record->entity_type)
+          ->condition('entity_id', $record->entity_id)

This is similar to the patch proposed, just a bit more consistent.

This could be correct, it copies all items that are published to the queue. Afterwards, bit by bit, it will validate if it can send the item to solr or not. If the status callback says no it will remove this item from the index (if exists) and it won't index the item again.

So, be careful with the re-index all button :-)

I don't think this is the right approach still.

We have duplicated code and we don't have any robustness in the case the call to delete from the index fails.

marking #1524988: Incorrect side effect in function apachesolr_index_get_entities_to_index() as duplicate, since this patch removes the side effect at least in getting rows to index.

This also adds an attempt at making the removal of an entity from the index more robust. A good test is to shut down solr and then delete or unpublish a node and see if it's eventually removed from the index when you start Solr up again later.

The path fix should go in a different issue/patch if it's not related.

Here's a more functional patch (i.e. I actually started testing it).

Ok, now I'm really confused as to why function apachesolr_get_index_bundles($env_id, $entity_type) depends on the env_id. We are, in general, deleting any excluded bundles from the tracking table, so it won't actually work to track indexing to 2 environemnts at once with different bundles allowed for each. This is a really big problem!

I think the correct answer is that we should not be adding/removing rows based on excluded bundles, since we already handle that in the query to get entities to index.

oops - that left a stray line in apachesolr_index_node_solr_reindex().

This fixes some logic errors in that the delete & reindex didn't rebuild the tracking table consistently with just marking all for reindex.

Because this removal from the table could mess our indexing process up I rewrote a small part of the batch code :

It now shows the user the following message :

"100 items successfully processed. 98 documents successfully sent to Solr."

Whenever you do the "Index remaining items" and it completes successfully.

Ah, It's not the cron :-)
In the admin UI there's a button : Index all queued content. This button starts a batch process that shows you that message. There is no bug in the cron process afaik.

Committed to dev. Couldn't reproduce the error Georgique mentions.

Committed to D6