Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added class check when deleteDocs in multishard setup #103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

added class check when deleteDocs in multishard setup #103

wants to merge 1 commit into from

Conversation

ilukac
Copy link

@ilukac ilukac commented Apr 2, 2013

In situation when there is more languages enabled in ezp but there are not all mapped to solr shards (multi-shard setup), object removal crashes. To prevent this we need to check if there is a shard mapped or not.

@@ -898,7 +898,8 @@ public function removeObjectById( $contentObjectId, $commit = null )
{
foreach ( $docs as $languageCode => $doc )
{
$this->SolrLanguageShards[$languageCode]->deleteDocs( array( $doc ), false, $commit, $optimize );
if ($this->SolrLanguageShards[$languageCode] instanceof eZSolrBase )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CS: space after "("

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpicker :P

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what ? ;-)

If this had been on one of the bigger repos, ezrobot would have said it, and not in a nice way ;-)

@bdunogier
Copy link
Member

Sounds overall like a bit of a crude fix. We should probably ensure that the language isn't added to SolrLanguageShards if it isn't mapped, but it may not be worth it at this point.

+1 with the CS fix.

Any counter opinion ? @paulborgermans ?

@paulborgermans
Copy link
Contributor

This looks like a crude fix, in the sense that it adds robustness towards ezfind.ini options for missing core mappings to languages. Now, if the language to core mapping is absent for some, it wont be indexed either afaik.

In case of a multi-core setup, it is still possible to assign the same core to multiple languages and this is what is intended (but maybe not documented enough).

@ivo: can you enlighten me how non-mapped languages are indexed in the first place? Isnt there another change/patch in use then which is omitted here?

@ilukac
Copy link
Author

ilukac commented Apr 3, 2013

Don't know about some other patch. It might exists as this problem was noted on 2012.6 version.
The site is old and used standard ezsearch and 4 languages before. Now we upgraded, enabled ezfind but using only 1 language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants