[
https://issues.apache.org/jira/browse/SOLR-11960?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16361679#comment-16361679
]
Tomás Fernández Löbbe commented on SOLR-11960:
----------------------------------------------
Thanks [~prusko], this looks very good. Some comments:
{code:java}
public Map<String,String> getCollectionProperties(final String collection) {
Map<String,String> properties = watchedCollectionProps.get(collection);
if (properties == null) {
try {
properties = fetchCollectionProperties(collection, null);
} catch (Exception e) {
throw new SolrException(ErrorCode.SERVER_ERROR, "Error reading
collection properties", e);
}
}
return properties;
}
{code}
I believe you are intentionally not adding the fetched properties back to the
{{watchedCollectionProps}} (because there is no watch, they would get stale).
Could you add a comment about it? I spent some time trying to figure out if
that was intentional or no. Also, could you add javadocs to this method?
In {{PropsWatcher.refreshAndWatch()}} you have this code:
{code:java}
} catch (KeeperException e) {
LOG.error("Unwatched collection: [{}]", coll, e);
{code}
What do you mean there with “Unwatched collection”?
If I understand correctly, on the first time someone calls
{{registerCollectionPropsWatcher}} for a particular zkStateReader and
collection, the watcher will be executed, but from then on, that’s not going to
happen again. Is this correct? If so, I believe we should make that consistent.
Components don’t necessarily know in which order they are loaded and this may
generate some strange behavior, also, multiple cores could belong to the same
collection in the same node and only the first one will trigger the watcher on
the registration. Maybe {{PropsWatcher.refreshAndWait()}} should receive a
boolean parameter and use that to know if it has to notify or not the watchers.
Then, when you are doing {{new PropsWatcher(collection).refreshAndWatch();}}
you’d use {{false}}, and {{true}} from {{PropsWatcher.process(…)}}. WDYT?
In {{refreshAndWatch()}} you are synchronizing on {{getUpdateLock()}}, which
sounds like overkill. I’m wondering if we really need any synchronization here
since we are just submitting tasks to a multi-thread Executor.
Should {{removeCollectionPropsWatcher}} also remove the collection from
{{collectionPropsWatches}} in case of {{canBeRemoved()==true}}?
Could you add a test that validates that collection properties are correctly
backed up and restored when using BACKUP and RESTORE APIs?
> Add collection level properties
> -------------------------------
>
> Key: SOLR-11960
> URL: https://issues.apache.org/jira/browse/SOLR-11960
> Project: Solr
> Issue Type: New Feature
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: Peter Rusko
> Priority: Major
> Attachments: 0001-SOLR-11960-add-collection-properties-support.patch,
> SOLR-11960.patch
>
>
> Solr has cluster properties, but no easy and extendable way of defining
> properties that affect a single collection. Collection properties could be
> stored in a single zookeeper node per collection, making it possible to
> trigger zookeeper watchers for only those Solr nodes that have cores of that
> collection.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
