[
https://issues.apache.org/jira/browse/CLOUDSTACK-8750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14803150#comment-14803150
]
ASF GitHub Bot commented on CLOUDSTACK-8750:
--------------------------------------------
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/778#issuecomment-141135958
Hi there @rafaelweingartner,
No hard feelings, dude.
Long story short:
* we are going through a critical phase with ACS - release
* I have been working on ACS since 2013, and it's always the same. We
stable master, we have a release and after that master is screwed again. After
6 months we go through this infinite loop of mess and nightmares.
* ACS is super uber tightly coupled, by the [bad] design it has. If I would
tell you that we had production problems because of 1 class of 1 hypervisor,
that we don't even use, was f*dup, you wouldn't believe me. So, any simple
change has the potential to make a huge mess. That's why the tests, etc.
* I respect your efforts and also the guys working with you. I will be an
student till I die!
If you can find a way to apply/submit your changes in an progressive way,
I'm more than glad to help.
Cheers,
Wilder
P.S.: adamantium claws, you know... I got to use them sometimes. ;)
> 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
> 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)
