Mailing-List: contact issues-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Date: Wed, 19 Aug 2015 17:04:50 +0000 (UTC)
From: "pedro henrique pereira martins (JIRA)" <jira@apache.org>
To: cloudstack-issues@incubator.apache.org
Message-ID: <JIRA.12857270.1440003832000.109058.1440003890035@Atlassian.JIRA>
In-Reply-To: <JIRA.12857270.1440003832000@Atlassian.JIRA>
References: <JIRA.12857270.1440003832000@Atlassian.JIRA>
 <JIRA.12857270.1440003832304@arcas>
Subject: =?utf-8?Q?[jira]_[Created]_(CLOUDSTACK-8750)_Change_?=
 =?utf-8?Q?the_variable_s=5Flogger_to_non-static_and_fi?=
 =?utf-8?Q?xed_its_name_in_=E2=80=9Ccom.cloud.utils.component?=
 =?utf-8?Q?.ComponentLifecycleBase=E2=80=9D_and_its_subclasses?=
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

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

             Summary: Change the variable s_logger to non-static and fixed =
its name in =E2=80=9Ccom.cloud.utils.component.ComponentLifecycleBase=E2=80=
=9D 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 defa=
ult.)
          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 =E2=80=9CComp=
onentLifecycleBase=E2=80=9D instantiate their on =E2=80=9Clogger=E2=80=9D m=
anually and uses a nonstandard name. We fixed that by changing the variable=
 in =E2=80=9CComponentLifecycleBase=E2=80=9D to protected and non-static an=
d instantiated it using the method =E2=80=9CgetClass=E2=80=9D from Object c=
lass. Therefore, we can reduce the code in a few hundred lines and use a mo=
re intuitive name for the logger variable.

We do understand that =E2=80=9Cs_ something=E2=80=9C is a proper way to ins=
tantiate a static variable in ACS classes. However, in few of the subclasse=
s of =E2=80=9Ccom.cloud.utils.component.ComponentLifecycleBase=E2=80=9D it =
was used names such as =E2=80=9CLOGGER=E2=80=9D (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 =E2=80=9Ccom.cloud.util=
s.component.ComponentLifecycleBase=E2=80=9D to protected and remove its sta=
tic and final declaration. Therefore we did the following in ComponentLifec=
ycleBase:

   protected Logger logger =3D Logger.getLogger(getClass());

  This way, every single subclass of ComponentLifecycleBase, when instantia=
ted would automatically have a logger instance for its proper class ready t=
o be used.

During that process we found a static method in class that used the =E2=80=
=9Cs_logger=E2=80=9D variable in classes:

com.cloud.network.element.VirtualRouterElement=20
org.apache.cloudstack.network.element.InternalLoadBalancerElement=20

To fix that we proposed to create a new class=20
=E2=80=9Ccom.cloud.network.element.HAProxyLBRule=E2=80=9D, instantiate it w=
ith @Componente and inject into the aforementioned classes.
That class will contain the following methods extracted from com.cloud.netw=
ork.element.VirtualRouterElement class:

com.cloud.network.element.HAProxyLBRule.containsOnlyNumbers(String, String)
com.cloud.network.element.HAProxyLBRule.validateHAProxyLBRule(LoadBalancing=
Rule)

However we don't know if all analyzed classes are singletons, if some of an=
alyzed classes aren't singletons. Therefore that change could have an impac=
t on memory consumption. It will require a deeper analysis to check which c=
lasses are singleton or not and leave the =E2=80=9Clogger=E2=80=9D as stati=
c in such classes.=C2=A0=20

We have a solution at our own branch=20
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, henc=
e that we have mapped all of ComponentLifecycleBase subclasses that use the=
 logger variable.


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