[
https://issues.apache.org/jira/browse/SOLR-14340?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17061758#comment-17061758
]
Erick Erickson commented on SOLR-14340:
---------------------------------------
[~dsmiley] It's not an expensive test though. True, I threw it in there on a
"while we're at it, let's check this too" basis.
I think a larger question is what the correct behavior should be. Upon
reflection, I don't like the fact that in this weird case we don't return any
information about this state, we just omit the collection altogether. The
collection exists, but requires some sleuthing to figure out what state it's in.
So the important bit is that in this weird case we do something rational and
return a clusterstatus that contains what makes sense. IIRC, we blew up before
(NPE?) and returned nothing useful at all.
ClusterStatus.java[154] has this comment:
{color:#808080}// skip this collection because the configset's znode has been
deleted
{color}{color:#808080}// which can happen during aggressive collection removal,
see SOLR-10720
{color}
and we skip adding the collection info entirely. Looking at 10720, I worry
about breaking something else if we returned a partial status, say the
collection with a configname of "WARNING_MISSING_CONFIGSET".
WDYT about adding an "errors" section to clusterstatus with a message like
"Configset for collection XY not found, status for collection omitted"?
Then we could remove all the rest of the checks in the readConfigName, I had no
idea it'd be that expensive. But I suspect as we work with more and more
collections, things like this will crop up.
I actually rather like the idea of an "errors" section to clusterstatus where
we can put anything weird we discover...
> ZkStateReader.readConfigName is doing too much work
> ---------------------------------------------------
>
> Key: SOLR-14340
> URL: https://issues.apache.org/jira/browse/SOLR-14340
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: David Smiley
> Assignee: David Smiley
> Priority: Major
>
> ZkStateReader.readConfigName reads the configSet name given a collection name
> parameter. Simple. It's doing too much work to do this though. First it's
> calling zkClient.exists() which is redundant given that we then call getData
> will will detect if it doesn't exist. Then we validate that the config path
> exists. But I don't think that should be verified on read, only on write.
> This method is a hotspot for nodes with lots of cores, proven out with
> profiling.
> Ideally the configSet name ought to be moved to state.json which is where all
> the other collection metadata is and is thus already read by most spots that
> want to read the configSet name. That would speed things up further and
> generally simplify things as well. That can happen on a follow-on issue, I
> think.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
