Comments

corvus_ch’s picture

Status: Active » Needs review
StatusFileSize
new4.03 KB

Here is a path that allows to exactly this either on a global or per index level.

corvus_ch’s picture

StatusFileSize
new5.97 KB

Here is an updated version that also prefixes the id in a more like this query.

corvus_ch’s picture

StatusFileSize
new6.03 KB

Patch did not apply anymore. Reroled.

drunken monkey’s picture

Status: Needs review » Needs work

I don't think we need per-index prefixes – after all, they'll already have different machine names. Or do you see/have a use case for this?

That said, the idea does make sense. If you only have one Solr server, but both production and development servers, this would probably be the easiest solution for having Solr for both.

Please also correct the following:

  • Properly document the hidden variable in README.txt.
  • Make getIndexId() public, and always pass the machine name instead of the complete object.
corvus_ch’s picture

Thanks fo the review.

I don't think we need per-index prefixes – after all, they'll already have different machine names. Or do you see/have a use case for this?

Yes I do have a use case. Imagine a site that has more than half a million indexed node. Adding a global prefix just because you ad one more index that might colide with other sites using the same Solr is not what you actually want to do. I agree that the use case is rare and probably other solutions exits but I like to have the option and actually it does not hurt to have both.

Improvements will follow.

arnested’s picture

I updated the patch based on @drunken_monkeys comments in #4.

I fixed a few minor code style issues in the new getIndexId() method.

I also renamed the variables from "search_api_solr_prefix" to "search_api_solr_index_prefix". I think adding "index" makes the names more precise.

I also backported the patch to 7.x-1.0-rc2 for those people stuck on that version (guess who's currently stuck ;-)

We need the patch for handling several development environments using the same Solr server.

drunken monkey’s picture

Status: Needs review » Needs work
+++ b/includes/service.inc
@@ -705,7 +706,7 @@ class SearchApiSolrService extends SearchApiAbstractService {
       $id = $this->createId($index->machine_name, $mlt['id']);
       $id = call_user_func(array($this->connection_class, 'phrase'), $id);
-      $keys = 'id:' . $id;
+      $keys = 'id:' . $this->createId($index_id, $mlt['id']);

I'm pretty sure you should replace $index->machine_name two lines above instead of that line.

Also, while the README.txt addition is pretty good already, it should contain a warning to only use alphanumeric characters and underscores, and a note to clear the index (or, better still, temporarily remove it from the server entirely) right before changing the variable. Otherwise, if you only re-index, the old data might be in there indefinitely.

arnested’s picture

You're right. I have changed the right occurrence of $index->machine_name instead.

I also added the warnings to the README.

New patch attached (and still a backport to 7.x-1.0-rc2 for those of us stuck in the past).

drunken monkey’s picture

Status: Needs review » Fixed

Thanks, this seems perfect. Committed.

arnested’s picture

Cool. Thank you.

Status: Fixed » Closed (fixed)

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