-
Notifications
You must be signed in to change notification settings - Fork 2.6k
SOLR-15094: Replace all code references of coreNodeName to replicaName #2234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| String msgNodeName = getNodeName(); | ||
| String msgCore = descriptor.getName(); | ||
|
|
||
| if (msgNodeName.equals(nodeName) && core.equals(msgCore)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NULLPTR_DEREFERENCE: accessing memory that is the null pointer on line 1710 indirectly during the call to ZkNodeProps.getStr(...).
merging with master
| this.collectionName = props.getProperty(CoreDescriptor.CORE_COLLECTION, coreName); | ||
| this.roles = props.getProperty(CoreDescriptor.CORE_ROLES, null); | ||
| this.nodeName = props.getProperty(CoreDescriptor.CORE_NODE_NAME); | ||
| this.nodeName = props.getProperty(REPLICA_NAME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename nodeName internally here as well? I think it's a little misleading
|
|
||
| /** | ||
| * See if coreNodeName has been taken over by another baseUrl and unload core | ||
| * See if replicaName has been taken over by another baseUrl and unload core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the log on line 80, and the variable name "cnn" as well
| private SolrZkClient zkClient; | ||
|
|
||
| public ElectionContext(final String coreNodeName, | ||
| public ElectionContext(final String relicaName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo :) missing p
| zkStateReader = zkController.getZkStateReader(); | ||
| baseUrl = zkController.getBaseUrl(); | ||
| coreZkNodeName = cd.getCloudDescriptor().getCoreNodeName(); | ||
| replicaName = cd.getCloudDescriptor().getCoreNodeName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update so we're not using deprecated method
| prepCmd.setCoreName(leaderCoreName); | ||
| prepCmd.setNodeName(zkController.getNodeName()); | ||
| prepCmd.setCoreNodeName(coreZkNodeName); | ||
| prepCmd.setCoreNodeName(replicaName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use new method
| public void newSearcher(SolrIndexSearcher newSearcher, SolrIndexSearcher currentSearcher) { | ||
| if (sleepTime.get() > 0) { | ||
| TestCloudSearcherWarming.coreNodeNameRef.set(newSearcher.getCore().getCoreDescriptor().getCloudDescriptor().getCoreNodeName()); | ||
| TestCloudSearcherWarming.replicaNameRef.set(newSearcher.getCore().getCoreDescriptor().getCloudDescriptor().getCoreNodeName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use getReplicaName
| CoreDescriptor.CORE_SHARD, r.getShard(), | ||
| CoreDescriptor.CORE_COLLECTION, r.getCollection(), | ||
| CoreDescriptor.CORE_NODE_NAME, r.getNodeName() | ||
| CommonParams.REPLICA_NAME, r.getNodeName() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r.getNodeName() should return the name of the node that the replica resides on. Use r.getName()
| } | ||
|
|
||
|
|
||
| @Deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super nit: keep getters/setters together :)
| cluster.waitForActiveCollection(COL, 1, 1); | ||
|
|
||
| // determine the coreNodeName of only current replica | ||
| // determine the replicaName of only current replica |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: rename variable on 895
| cluster.waitForActiveCollection(COL, 1, 1); | ||
|
|
||
| // determine the coreNodeName of only current replica | ||
| // determine the replicaName of only current replica |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Rename variable on 888
This PR is just focusing on code level changes.
The actual external facing variables can be addressed separately