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.vit...@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.zh...@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:nore...@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
> >> >
> >> >
> >

Reply via email to