[jira] [Commented] (SOLR-8250) remove member/variable shadowing in ElectionContext

["Ishan Chattopadhyaya (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SOLR-8250?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14995020#comment-14995020 ] 

Ishan Chattopadhyaya commented on SOLR-8250:
--------------------------------------------

bq. Was looking to change some logging in this file and stumbled across this.
Hi Christine, I wanted to make a minor logging change to ElectionContext. When you're looking at the logging tweaks, can you please club this together?
{noformat}
Index: solr/core/src/java/org/apache/solr/cloud/ElectionContext.java
===================================================================
--- solr/core/src/java/org/apache/solr/cloud/ElectionContext.java	(revision 1713022)
+++ solr/core/src/java/org/apache/solr/cloud/ElectionContext.java	(working copy)
@@ -182,7 +182,7 @@
         
         @Override
         public void execute() throws InterruptedException, KeeperException {
-          log.info("Creating leader registration node", leaderPath);
+          log.info("Creating leader registration node: {}", leaderPath);
           List<Op> ops = new ArrayList<>(2);
           
           // We use a multi operation to get the parent nodes version, which will
{noformat}

> remove member/variable shadowing in ElectionContext
> ---------------------------------------------------
>
>                 Key: SOLR-8250
>                 URL: https://issues.apache.org/jira/browse/SOLR-8250
>             Project: Solr
>          Issue Type: Wish
>    Affects Versions: Trunk
>            Reporter: Christine Poerschke
>         Attachments: SOLR-8250.patch
>
>
> Was looking to change some logging in this file and stumbled across this. Attaching patch to illustrate and describing in words here also.
> #1 {{OverseerElectionContext.runLeaderProcess}} has a local variable {{id}} that shadows the {{id}} member in the {{ElectionContext}} base class.
> #2 So i went and renamed {{id}} to {{coreNodeName}} (the {{ElectionContext}} constructor uses that as its argument name already and thus {{this.id = coreNodeName;}} becomes {{this.coreNodeName = coreNodeName;}}, so far, so good.
> #3 Next then i noticed that the {{ShardLeaderElectionContext.startLeaderInitiatedRecoveryOnReplicas}} local variable {{coreNodeName}} is now shadowing {{ElectionContext}}'s coreNodeName member and that on the preceding line local variable shardId already shadows the {{shardId}} member in {{ShardLeaderElectionContextBase}} class.
> #4 So then to remove the shadowing i renamed the local variables giving {{cloudDescCollection}} (formerly coll), {{cloudDescShardId}} (formerly shardId) and {{cloudDescCoreNodeName}} (formerly coreNodeName) as the local variables with {{ShardLeaderElectionContextBase}}'s members still being called {{collection}} and {{shardId}} plus the {{coreNodeName}} member from the {{ElectionContext}} base class.
> #5 This then highlights when the local variables are used and when the (now no longer shadowed) class members are used and we have a mix with
> {code}
> -      String znodePath = zkController.getLeaderInitiatedRecoveryZnodePath(coll, shardId);
> +      String znodePath = zkController.getLeaderInitiatedRecoveryZnodePath(cloudDescCollection, cloudDescShardId);
> {code}
> and
> {code}
> -          final Replica.State lirState = zkController.getLeaderInitiatedRecoveryState(coll, shardId, replicaCoreNodeName);
> +          final Replica.State lirState = zkController.getLeaderInitiatedRecoveryState(cloudDescCollection, cloudDescShardId, replicaCoreNodeName);
> {code}
> using the local variables but
> {code}
>              List<ZkCoreNodeProps> replicaProps =  
> -                zkController.getZkStateReader().getReplicaProps(collection, shardId, coreNodeName);
> +                zkController.getZkStateReader().getReplicaProps(collection, cloudDescShardId, cloudDescCoreNodeName);
> {code}
> and
> {code}        
>                zkController.ensureReplicaInLeaderInitiatedRecovery(cc,
> -                  collection, shardId, coreNodeProps, core.getCoreDescriptor(),
> +                  collection, cloudDescShardId, coreNodeProps, core.getCoreDescriptor(),
>                    false /* forcePublishState */);
> {code}
> using the collection member but the shard id and core node name local variables.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org