Gus Heck created SOLR-13544:
-------------------------------
Summary: Consider adding isCloud() method as shortcut for
cloudDescriptor == null checks
Key: SOLR-13544
URL: https://issues.apache.org/jira/browse/SOLR-13544
Project: Solr
Issue Type: Improvement
Security Level: Public (Default Security Level. Issues are Public)
Components: SolrCloud
Reporter: Gus Heck
In writing SOLR-13420 I was not supper happy with the verbosity of some of my
changes, and I think a convenience method could improve code legibility.
After doing some searching I found that in the first 50ish places where
getCloudDescriptor() is called. 8 of them are followed by a null check, which
is generally necessitated in cases where the code might run in non-cloud
environments. There are a great many places where null is not checked and cloud
is assumed, presumably for reasons related to the nature of the code (i.e.
cdcr, code in zkcontroller), but some code does check, and the ethos generally
seems to be "do x if we're in cloud".
For example in MDCLoggingContext.java I see:
{code:java}
// If this is a replica of type PULL, don't create the update log
boolean skipUpdateLog = core.getCoreDescriptor().getCloudDescriptor() !=
null && !core.getCoreDescriptor().getCloudDescriptor().requiresTransactionLog();
{code}
This would be more concise and readable as:
{code:java}
boolean skipUpdateLog = core.isCloud() &&
!core.getCoreDescriptor().getCloudDescriptor().requiresTransactionLog();
{code}
Or in the routed alias code:
{code:java}
SolrCore core = req.getCore();
CoreDescriptor coreDescriptor = core.getCoreDescriptor();
CloudDescriptor cloudDescriptor = coreDescriptor.getCloudDescriptor();
if (cloudDescriptor != null) {
String collectionName = cloudDescriptor.getCollectionName();
CoreContainer coreContainer = core.getCoreContainer();
ZkController zkController = coreContainer.getZkController();
... etc ...{code}
vs this which seems to read much smoother...
{code:java}
SolrCore core = req.getCore();
if (core.isCloud()) {
CoreDescriptor coreDescriptor = core.getCoreDescriptor();
CloudDescriptor cloudDescriptor = coreDescriptor.getCloudDescriptor();
String collectionName = cloudDescriptor.getCollectionName();
... etc ...
{code}
The idea would be to add to SolrCore this method
{code:java}
public boolean isCloud() {
return getCoreDescriptor().getCloudDescriptor() != null;
} {code}
We also might add some asserts in there for other things folks might want to
assume to be true in the cloud case and javadoc what's reliable when this
returns true, but keep it skinny in production. The key question is are there
any cases where we're in solr cloud but getCloudDescriptor() is null? Seems
hard to imagine, but what do others think?
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
