[
https://issues.apache.org/jira/browse/SOLR-7374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15331892#comment-15331892
]
Varun Thacker commented on SOLR-7374:
-------------------------------------
Hi Hrishikesh,
Patch is looking good!
Here are my main two concerns
I think we need to deal with the "location" param better. Before this patch we
used to read location as a query param . If the query param is empty then we
read it from the cluster property.
With this patch we are adding the ability to specify "location" in the solr.xml
file but it will never be used? CollectionsHandler will bail out early today .
{code}
String location = req.getParams().get("location");
if (location == null) {
location =
h.coreContainer.getZkController().getZkStateReader().getClusterProperty("location",
(String) null);
}
if (location == null) {
throw new SolrException(ErrorCode.BAD_REQUEST, "'location' is not
specified as a query parameter or set as a cluster property");
}
{code}
One approach would be to deprecate the usage of cluster prop and look at query
param followed by solr.xml ? Looking at three places seems messy .
{{repo}} is the key used to specify the implementation. In Solr xml the tag is
called {{repository}} . Should we just use repository throughout?
Small changes:
- Javadocs for BackupRepository - s/index/indexes
- I think we should follow the {{if (condition)}} spacing convention ? Some of
the places don't have spaceIn and some do.
- In {{BackupRepositoryFactory}} : In these two log log lines can we mention
the name as well - {{LOG.info("Default configuration for backup repository is
with configuration params {}", defaultBackupRepoPlugin);}} and
{{LOG.info("Added backup repository with configuration params {}",
backupRepoPlugins\[i]);}}
- Can we reuse the "location" string with
(BackupRepository.DEFAULT_LOCATION_PROPERTY) on line 871/925 of
CoreAdminOperation? Let's fix it in CollectionsHandler and
OverseerCollectionMessageHandler as well?
- In RestoreCore do we need to deprecate the old RestoreCore ctor ? Any reason
why we can't remove it directly?
> Backup/Restore should provide a param for specifying the directory
> implementation it should use
> -----------------------------------------------------------------------------------------------
>
> Key: SOLR-7374
> URL: https://issues.apache.org/jira/browse/SOLR-7374
> Project: Solr
> Issue Type: Bug
> Reporter: Varun Thacker
> Assignee: Mark Miller
> Fix For: 5.2, 6.0
>
> Attachments: SOLR-7374.patch, SOLR-7374.patch, SOLR-7374.patch,
> SOLR-7374.patch
>
>
> Currently when we create a backup we use SimpleFSDirectory to write the
> backup indexes. Similarly during a restore we open the index using
> FSDirectory.open .
> We should provide a param called {{directoryImpl}} or {{type}} which will be
> used to specify the Directory implementation to backup the index.
> Likewise during a restore you would need to specify the directory impl which
> was used during backup so that the index can be opened correctly.
> This param will address the problem that currently if a user is running Solr
> on HDFS there is no way to use the backup/restore functionality as the
> directory is hardcoded.
> With this one could be running Solr on a local FS but backup the index on
> HDFS etc.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
