Mailing-List: contact issues-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Date: Tue, 15 Sep 2015 15:07:46 +0000 (UTC)
From: "ASF GitHub Bot (JIRA)" <jira@apache.org>
To: cloudstack-issues@incubator.apache.org
Message-ID: <JIRA.12857270.1440003832000.342913.1442329666157@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]_[Commented]_(CLOUDSTACK-8750)_Chang?=
 =?utf-8?Q?e_the_variable_s=5Flogger_to_non-static_and_f?=
 =?utf-8?Q?ixed_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


    [ https://issues.apache.org/jira/browse/CLOUDSTACK-8750?page=3Dcom.atla=
ssian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=
=3D14745579#comment-14745579 ]=20

ASF GitHub Bot commented on CLOUDSTACK-8750:
--------------------------------------------

Github user rafaelweingartner commented on the pull request:

    https://github.com/apache/cloudstack/pull/778#issuecomment-140423091
 =20
    @wilderrodrigues conflicts solved, I tried to do a maven install, to ru=
n all tests and compile everything, but that did not work, hence there is a=
 class missing "com.cloud.network.schema.showvcs.Output;"
   =20
    @miguelaferreira I think the commit heading is better now


> 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 subcla=
sses
> -------------------------------------------------------------------------=
-------------------------------------------------------------
>
>                 Key: CLOUDSTACK-8750
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-875=
0
>             Project: CloudStack
>          Issue Type: Improvement
>      Security Level: Public(Anyone can view this level - this is the defa=
ult.)=20
>          Components: Management Server
>            Reporter: pedro henrique pereira martins
>            Assignee: pedro henrique pereira martins
>            Priority: Minor
>             Fix For: Future
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> We have noticed that every single class that is a subclass of =E2=80=9CCo=
mponentLifecycleBase=E2=80=9D instantiate their on =E2=80=9Clogger=E2=80=9D=
 manually and uses a nonstandard name. We fixed that by changing the variab=
le in =E2=80=9CComponentLifecycleBase=E2=80=9D to protected and non-static =
and instantiated it using the method =E2=80=9CgetClass=E2=80=9D 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 =E2=80=9Cs_ something=E2=80=9C is a proper way to i=
nstantiate a static variable in ACS classes. However, in few of the subclas=
ses of =E2=80=9Ccom.cloud.utils.component.ComponentLifecycleBase=E2=80=9D i=
t was used names such as =E2=80=9CLOGGER=E2=80=9D (that is a proper name fo=
r static field as JAVA standards), log, status_logger and others.
> What we propose is to change the logger variable in =E2=80=9Ccom.cloud.ut=
ils.component.ComponentLifecycleBase=E2=80=9D to protected and remove its s=
tatic and final declaration. Therefore we did the following in ComponentLif=
ecycleBase:
>    protected Logger logger =3D Logger.getLogger(getClass());
>   This way, every single subclass of ComponentLifecycleBase, when instant=
iated 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 =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=
 with @Componente and inject into the aforementioned classes.
> That class will contain the following methods extracted from com.cloud.ne=
twork.element.VirtualRouterElement class:
> com.cloud.network.element.HAProxyLBRule.containsOnlyNumbers(String, Strin=
g)
> com.cloud.network.element.HAProxyLBRule.validateHAProxyLBRule(LoadBalanci=
ngRule)
> 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 imp=
act on memory consumption. It will require a deeper analysis to check which=
 classes are singleton or not and leave the =E2=80=9Clogger=E2=80=9D as sta=
tic in such classes.=C2=A0=20
> We have a solution at our own branch=20
> https://github.com/rafaelweingartner/cloudstack/tree/master-lrg-cs-hackda=
y-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, he=
nce that we have mapped all of ComponentLifecycleBase subclasses that use t=
he logger variable.


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