Hi all guys, sorry to be late but I had to read all the resume before posting something useful :P So, briefly, just my 2 cents with the hope to contribute in a useful way:
About the JMX support, that could involve also the whole design, I suggest on keeping out the Configuration but rather taking in consideration only the Pool/Factory classes, avoid to add the getConfig() method. Moreover, I'd prefer keeping the Config reference in the Pool/Factory and not storing the config preferences, to avoid * data redundancy; * boilerplate code to copy Config -> Pool/Factory properties. When I started thinking about how to refactor the config, I had in mind the HttpClient 3 preferences, where data is not stored directly in the HttpClient instance, but rather in the Params object, then reused in the HttpClient itself. Of course there are pros and cons, at the moment of the refactor I was more focused on * simpler design respect the Pool 1.X; * remove data redundancies. That's why I wouldn't follow the option #2; but please, explain me which are the side effects of that design, so I can avoid to repeat the same mistake in the future. Many thanks in advance, have a nice day!!! Simo http://people.apache.org/~simonetripodi/ http://www.99soft.org/ On Mon, Oct 25, 2010 at 6:43 PM, Phil Steitz <phil.ste...@gmail.com> wrote: > On 10/25/10 12:36 PM, James Carman wrote: >> >> On Mon, Oct 25, 2010 at 12:25 PM, Phil Steitz<phil.ste...@gmail.com> >> wrote: >>> >>> I notice now what I missed on initial review of Simo's patch - the pool >>> accessors now manage the config properties via persisted Config members. >>> I >>> am OK with this, but it now means that the Config classes have to be >>> mutable. What needs to be threadsafe, however, is the pool itself. Given >>> that you can't rely on Config locks to ensure correctness for the pool >>> (i.e. >>> the pool-exposed mutators are still going to have to lock the pool >>> itself), >>> making Config accessors synchronized is just adding extra synch. >>> >> >> I haven't had a chance to review the proposed code changes yet, but >> why not just use a reconfigure(Config) method on the pool objects >> which is called by the constructor and by the outside world (including >> JMX stuff)? There are two options with this approach: >> >> 1. Make a copy of the Config object so that outside changes (if >> Config is left mutable) don't make an impact. >> 2. Don't refer directly to the Config object at all inside the pool's >> implementation. You'd have to copy the config information somewhere >> else obviously. > > I like 2. better. That is how it worked before (pool properties were held in > individual fields). I missed that part of the change. Not all properties > are mutable and we want to be able to set / get individual properties at > runtime. That would be awkward under option 1. > > Phil > >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
