[jira] [Created] (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>]

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

             Summary: 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
            Priority: Minor
             Fix For: Future


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)