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.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 > > >> > > > >> > > > >
