incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Koushik Das <koushik....@citrix.com>
Subject RE: Review Request: CLOUDSTACK-810: Make DirectAgent thread pool size configurable
Date Thu, 10 Jan 2013 11:39:41 GMT
So is it ok to leave it like this for now and revisit after Kelven's changes are in. Or should
I move it in AgentManagerImpl?

-Koushik

> -----Original Message-----
> From: Frank Zhang
> Sent: Thursday, January 10, 2013 6:46 AM
> To: CloudStack DeveloperList; 'Chiradeep Vittal'; Abhinandan Prateek; Alex
> Huang
> Cc: Koushik Das
> Subject: RE: Review Request: CLOUDSTACK-810: Make DirectAgent thread
> pool size configurable
> 
> Yeah I agree.
> As Kelven is replacing injection with Spring, we are eased from this pain.
> Spring supports init function which guarantees be called after all
> dependencies of this bean being loaded.
> Then we can inject configDao in AgentManagerImpl and initialize the value in
> a init().
> This avoid changing current componentloader to make ConfigDao load
> before AgentManager
> 
> > -----Original Message-----
> > From: Chiradeep Vittal [mailto:Chiradeep.Vittal@citrix.com]
> > Sent: Wednesday, January 09, 2013 5:08 PM
> > To: CloudStack DeveloperList; 'Chiradeep Vittal'; Abhinandan Prateek;
> > Alex Huang
> > Cc: Koushik Das
> > Subject: Re: Review Request: CLOUDSTACK-810: Make DirectAgent thread
> > pool size configurable
> >
> > You may be right about the specific order of things in this particular
> > case, but we shouldn't assume that the order of initialization is
> > going to be the same forever.
> >
> > On 1/9/13 4:59 PM, "Frank Zhang" <Frank.Zhang@citrix.com> wrote:
> >
> > >Though I agree we can put it in initialization code of
> > >AgentManagerImpl, we have to guarantee AgentManagerImpl  is loaded
> > before ConfigDao.
> > >Technically I think it's ok to the Dao is guaranteed before this
> > >static block of DirectoAgentAttache.
> > >
> > >A static block is invoked when a class type initialized, and from
> > >Java spec, a class type is initialized when
> > >
> > >>12.4.1. When Initialization Occurs
> > >
> > >>A class or interface type T will be initialized immediately before
> > >>the first occurrence of any one of the following:
> > >
> > >>T is a class and an instance of T is created.
> > >
> > >>T is a class and a static method declared by T is invoked.
> > >
> > >>A static field declared by T is assigned.
> > >
> > >>A static field declared by T is used and the field is not a constant
> > >>variable (§4.12.4).
> > >
> > >>T is a top level class (§7.6), and an assert statement (§14.10)
> > >>lexically nested within T (§8.1.3) is executed.
> > >
> > >>A reference to a static field (§8.3.1.1) causes initialization of
> > >>only the class or interface that actually declares it, even though
> > >>it might be referred to through the name of a subclass, a
> > >>subinterface, or a class that implements an interface.
> > >
> > >>Invocation of certain reflective methods in class Class and in
> > >>package java.lang.reflect also causes class or interface initialization.
> > >
> > >For Dao is loaded when componentloader initialized, at that time
> > >being we don't have code path pertaining to DirectAgentAttach class.
> > >
> > >
> > >
> > >
> > >> -----Original Message-----
> > >> From: Chiradeep Vittal [mailto:noreply@reviews.apache.org] On
> > >> Behalf Of Chiradeep Vittal
> > >> Sent: Wednesday, January 09, 2013 4:39 PM
> > >> To: Abhinandan Prateek; Alex Huang
> > >> Cc: cloudstack; Koushik Das; Chiradeep Vittal
> > >> Subject: Re: Review Request: CLOUDSTACK-810: Make DirectAgent
> > thread
> > >> pool size configurable
> > >>
> > >>
> > >> -----------------------------------------------------------
> > >> This is an automatically generated e-mail. To reply, visit:
> > >> https://reviews.apache.org/r/8855/#review15219
> > >> -----------------------------------------------------------
> > >>
> > >>
> > >>
> > >> server/src/com/cloud/agent/manager/DirectAgentAttache.java
> > >> <https://reviews.apache.org/r/8855/#comment32827>
> > >>
> > >>     We cannot assume that the Daos will be loaded before this
> > >>static initializer  is called. The order of calling static
> > >>initializers is not defined. It is better to  initialize it via the
> > >>AgentManagerImpl
> > >>
> > >>
> > >> - Chiradeep Vittal
> > >>
> > >>
> > >> On Jan. 8, 2013, 8:06 a.m., Koushik Das wrote:
> > >> >
> > >> > -----------------------------------------------------------
> > >> > This is an automatically generated e-mail. To reply, visit:
> > >> > https://reviews.apache.org/r/8855/
> > >> > -----------------------------------------------------------
> > >> >
> > >> > (Updated Jan. 8, 2013, 8:06 a.m.)
> > >> >
> > >> >
> > >> > Review request for cloudstack, Abhinandan Prateek and Alex Huang.
> > >> >
> > >> >
> > >> > Description
> > >> > -------
> > >> >
> > >> > Currently the DirectAgent pool size is hard-coded to 500. One of
> > >> > the
> > >>factors
> > >> that can affect this is the number of hosts in a deployment. If
> > >>there are more  than 500 hosts (say around 1K) then this pool can
> > >>easily get exhausted  resulting in delays and undesired behavior.
> > >> >
> > >> > Removed hard-coding of directagent thread pool size and now
> > >> > reading it
> > >> from configuration.
> > >> >
> > >> >
> > >> > This addresses bug CLOUDSTACK-810.
> > >> >
> > >> >
> > >> > Diffs
> > >> > -----
> > >> >
> > >> >   server/src/com/cloud/agent/manager/DirectAgentAttache.java
> > 848c7e6
> > >> >   server/src/com/cloud/configuration/Config.java 92313ea
> > >> >   setup/db/db/schema-40to410.sql 4455956
> > >> >
> > >> > Diff: https://reviews.apache.org/r/8855/diff/
> > >> >
> > >> >
> > >> > Testing
> > >> > -------
> > >> >
> > >> > Verified that configuration entry is present in the 'configuration'
> > >>table.
> > >> > Also verified in a debugger that it is read correctly while
> > >> > creating
> > >>the
> > >> directagent thread pool.
> > >> >
> > >> >
> > >> > Thanks,
> > >> >
> > >> > Koushik Das
> > >> >
> > >> >
> > >


Mime
View raw message