mbwaheed commented on a change in pull request #1223: SOLR-14213: Configuring
Solr Cloud to use Shared Storage
URL: https://github.com/apache/lucene-solr/pull/1223#discussion_r381674933
##########
File path:
solr/core/src/java/org/apache/solr/store/shared/SharedStoreManager.java
##########
@@ -43,68 +43,38 @@
public SharedStoreManager(ZkController controller) {
zkController = controller;
- // initialize BlobProcessUtil with the SharedStoreManager for background
processes to be ready
- blobProcessUtil = new BlobProcessUtil(zkController.getCoreContainer());
- blobCoreSyncer = new BlobCoreSyncer();
- sharedCoreConcurrencyController = new
SharedCoreConcurrencyController(zkController.getCoreContainer());
- }
-
- @VisibleForTesting
- public void initBlobStorageProvider(BlobStorageProvider blobStorageProvider)
{
- this.blobStorageProvider = blobStorageProvider;
- }
-
- @VisibleForTesting
- public void initBlobProcessUtil(BlobProcessUtil processUtil) {
- if (blobProcessUtil != null) {
- blobProcessUtil.shutdown();
- }
- blobProcessUtil = processUtil;
+ blobStorageProvider = new BlobStorageProvider();
+ blobDeleteManager = new
BlobDeleteManager(getBlobStorageProvider().getClient());
+ corePullTracker = new CorePullTracker();
+ sharedShardMetadataController = new
SharedShardMetadataController(zkController.getSolrCloudManager());
+ sharedCoreConcurrencyController = new
SharedCoreConcurrencyController(sharedShardMetadataController);
}
- /*
- * Initiates a SharedShardMetadataController if it doesn't exist and returns
one
+ /**
+ * Start blob processes that depend on an initiated SharedStoreManager
*/
+ public void load() {
+ blobCoreSyncer = new BlobCoreSyncer();
Review comment:
It is not clear why BlobCoreSyncer is initialized here inside load method
instead of the constructor.
The comment says things here depend on initiated SharedStoreManager. That
statement is incomplete. It is that plus the CoreContainer#sharedStoreManager
is initialized because CorePullerFeeder depends on that. Also blobCoreSyncer
and blobProcessUtil are two fields that are not initialized yet. The comment
suggests all the public facing fields would have been initialized by now. Even
if that is not what the comment means, there is nothing preventing them from
being accessed in null state.
The other problem is that if the CoreContainer#load fails then
CoreContainer@shutdown is called for cleanup. At least inside
CoreContainer#createAndLoad. That can hit an NPE when calling
sharedStoreManager.getBlobProcessManager().shutdown() because it is possible
the exception that started the shutdown was in sharedStoreManager#load before
blobProcessUtil was even initialized.
The second problem can be solved by creating a shutdown method inside
SharedStoreManager and moving
sharedStoreManager.getBlobProcessManager().shutdown() there, protected by
blobProcessUtil!= null check.
Though, I don't have any good ideas for the first problem.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
