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)
