cloudstack-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "pedro henrique pereira martins (JIRA)" <j...@apache.org>
Subject [jira] [Reopened] (CLOUDSTACK-8750) Change the variable s_logger to non-static and fixed its name in “com.cloud.utils.component.ComponentLifecycleBase” and its subclasses
Date Wed, 16 Sep 2015 15:17:46 GMT

     [ https://issues.apache.org/jira/browse/CLOUDSTACK-8750?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

pedro henrique pereira martins reopened CLOUDSTACK-8750:
--------------------------------------------------------

> 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
>   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)

Mime
View raw message