-
Notifications
You must be signed in to change notification settings - Fork 34
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
EZP-30463: As an Developer, I want to use Solr Cloud #137
Conversation
☁️FTW😉 |
.travis.yml
Outdated
@@ -18,7 +18,8 @@ matrix: | |||
env: TEST_CONFIG="phpunit-integration-legacy-solr.xml" SOLR_VERSION="6.5.1" CORES_SETUP="shared" | |||
- php: 7.2 | |||
env: TEST_CONFIG="phpunit-integration-legacy-solr.xml" SOLR_VERSION="6.6.5" CORES_SETUP="single" SOLR_CORES="collection1" | |||
|
|||
- php: 7.2 | |||
env: TEST_CONFIG="phpunit-integration-legacy-solr.xml" SOLR_VERSION="6.6.5" CORES_SETUP="cloud" SOLR_CORES="collection1" SOLR_CLOUD="yes" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, based on logic in getTestConfigurationFile it does not seems like we need both CORES_SETUP and SOLR_CLOUD. CORES_SETUP=cloud
seems like it could be detected anywhere needed.
But maybe logic in getTestConfigurationFile is temprary (simple for now) and you intend to allow several different cloud configs right?
bundle/DependencyInjection/EzSystemsEzPlatformSolrSearchEngineExtension.php
Outdated
Show resolved
Hide resolved
lib/Gateway/DistributionStrategy/DocumentRouter/NullDocumentRouter.php
Outdated
Show resolved
Hide resolved
a1818cf
to
7a2d376
Compare
PR has been updated with significant changes so please re-review 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one small thing which we discussed before. It seems that it has to be covered in this place.
Besides, looks good to me, great job! 🙂
copy_files ${TEMPLATE_DIR} "${files[*]}" | ||
|
||
# modify solrconfig.xml to remove section that doesn't agree with our schema | ||
sed -i.bak '/<updateRequestProcessorChain name="add-unknown-fields-to-the-schema">/,/<\/updateRequestProcessorChain>/d' "${TEMPLATE_DIR}/solrconfig.xml" || exit_on_error "Can't modify file '${TEMPLATE_DIR}/solrconfig.xml'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you have to add this line (https://github.com/ezsystems/ezplatform-solr-search-engine/blob/master/bin/generate-solr-config.sh#L123) here as well even though tests are passing. Without it, you'll notice a significant delay before seeing content changes (eg. breadcrumbs in AdminUI).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied in 260c729
download | ||
|
||
if [ "$SOLR_CLOUD" = "no" ]; then | ||
$SCRIPT_DIR/../generate-solr-config.sh \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamwojs This script is used to generate config for platform.sh, which is kind of a must. So most likely many of the changes here should be moved into generate-solr-config.s
, so script will be able to continued to be used also with cloud config on platform.sh.
@vidarl might be able to provide more info, and also who to talk to there if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be right, but let's process with combination platform.sh + Solr Cloud as follow up for this PR. I want to unblock QA on this PR and platform.sh itself is a quite new topic for me 😉
composer.json
Outdated
@@ -14,7 +14,7 @@ | |||
"prefer-stable": true, | |||
"require": { | |||
"php": "^7.1", | |||
"ezsystems/ezpublish-kernel": "^8.0@dev", | |||
"ezsystems/ezpublish-kernel": "dev-ezp_30463_solr_cloud as 7.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ezsystems/ezpublish-kernel": "dev-ezp_30463_solr_cloud as 7.5", | |
"ezsystems/ezpublish-kernel": "^7.5.2@dev || ^8.0@dev", |
Once kernel pr is merged.
@adamwojs If this is to work with eZ Platform 2.5 LTS you need to rebase against the latest stable branch, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cloud seems to work fine in various configurations of nodes, shards, and cores. Searching can be performed for various languages (japanese, polish, french, german, english). Searching still works after deleting the shard replica. After deleting Leader replica, another one is taking its place automatically. Documents are distributed quasi equally (distribution scales well with the number of documents).
QA approves
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 But please push 1.7 branch based on 1.6 + composer changes and rebase this on that before merging, as @alongosz said master is 3.0 only now and we need this on 2.5.
PR has been rebased against the new 1.7 branch (also temporary composer.json changes has been reverted). Last successful CI build https://travis-ci.org/ezsystems/ezplatform-solr-search-engine/builds/548644319 |
Good job @adamwojs! It's cool to see this feature 🎉 |
Description
This PR introduces support for Solr Cloud. More information https://lucene.apache.org/solr/guide/6_6/solrcloud.html
Configuration
From now on the user can specify data distribution strategy for connection via
distribution_strategy
option. The possible values are:standalone
- https://lucene.apache.org/solr/guide/6_6/legacy-scaling-and-distribution.html (current implementation)cloud
- Solr CloudThe default value is
standalone
for backward compatibility reasons.Example Solr Cloud configuration :
Cluster screenshot:
Document routing
This solution uses the default Solr Cloud document routing strategy:
compositeId
. In compare toimplicit
strategy eZ Platform doesn't need to know shards list.More information: https://lucene.apache.org/solr/guide/6_6/shards-and-indexing-data-in-solrcloud.html#ShardsandIndexingDatainSolrCloud-DocumentRouting
Language specific analysis
The configuration is based on Multi-Core setup so any specific language analysis options could be specified on the collection level.