[jira] [Comment Edited] (CLOUDSTACK-8750) Change the variable s_logger to non-static and fixed its name in “com.cloud.utils.component.ComponentLifecycleBase” and its subclasses

["pedro henrique pereira martins (JIRA)" <ji...@apache.org>]

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

pedro henrique pereira martins edited comment on CLOUDSTACK-8750 at 8/29/15 8:24 PM:
-------------------------------------------------------------------------------------

will be change in version 4.7


was (Author: pedro-martins):
will be changed in version 4.7

> Change the variable s_logger to non-static and fixed its name in “com.cloud.utils.component.ComponentLifecycleBase” and its subclasses
> --------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CLOUDSTACK-8750
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-8750
>             Project: CloudStack
>          Issue Type: Improvement
>      Security Level: Public(Anyone can view this level - this is the default.) 
>          Components: Management Server
>            Reporter: pedro henrique pereira martins
>            Assignee: pedro henrique pereira martins
>            Priority: Minor
>             Fix For: Future
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> We have noticed that every single class that is a subclass of “ComponentLifecycleBase” instantiate their on “logger” manually and uses a nonstandard name. We fixed that by changing the variable in “ComponentLifecycleBase” to protected and non-static and instantiated it using the method “getClass” from Object class. Therefore, we can reduce the code in a few hundred lines and use a more intuitive name for the logger variable.
> We do understand that “s_ something“ is a proper way to instantiate a static variable in ACS classes. However, in few of the subclasses of “com.cloud.utils.component.ComponentLifecycleBase” it was used names such as “LOGGER” (that is a proper name for static field as JAVA standards), log, status_logger and others.
> What we propose is to change the logger variable in “com.cloud.utils.component.ComponentLifecycleBase” to protected and remove its static and final declaration. Therefore we did the following in ComponentLifecycleBase:
>    protected Logger logger = Logger.getLogger(getClass());
>   This way, every single subclass of ComponentLifecycleBase, when instantiated would automatically have a logger instance for its proper class ready to be used.
> During that process we found a static method in class that used the “s_logger” variable in classes:
> com.cloud.network.element.VirtualRouterElement 
> org.apache.cloudstack.network.element.InternalLoadBalancerElement 
> To fix that we proposed to create a new class 
> “com.cloud.network.element.HAProxyLBRule”, instantiate it with @Componente and inject into the aforementioned classes.
> That class will contain the following methods extracted from com.cloud.network.element.VirtualRouterElement class:
> com.cloud.network.element.HAProxyLBRule.containsOnlyNumbers(String, String)
> com.cloud.network.element.HAProxyLBRule.validateHAProxyLBRule(LoadBalancingRule)
> However we don't know if all analyzed classes are singletons, if some of analyzed classes aren't singletons. Therefore that change could have an impact on memory consumption. It will require a deeper analysis to check which classes are singleton or not and leave the “logger” as static in such classes.  
> We have a solution at our own branch 
> https://github.com/rafaelweingartner/cloudstack/tree/master-lrg-cs-hackday-003
> However, there is still the need to double-check the singleton problem.
> There is a PR we created in our first attempt to solve that problem:
> https://github.com/apache/cloudstack/pull/714
> The dev that starts working on this ticket can reach us at any moment, hence that we have mapped all of ComponentLifecycleBase subclasses that use the logger variable.



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