On 15 December 2011 08:26, Mark Thomas <ma...@apache.org> wrote: > On 14/12/2011 23:12, sebb wrote: >> On 14 December 2011 22:22, Mark Thomas <ma...@apache.org> wrote: >>> On 14/12/2011 19:12, sebb wrote: >>>> On 14 December 2011 18:29, Mark Thomas <ma...@apache.org> wrote: >>>>> JNDI assumes resources are essentially beans i.e. have zero argument >>>>> constructors and getters/setters. If this is not the case then some >>>>> extra plumbing is required. The further G[K]OP gets from a JavaBean the >>>>> more plumbing required. Currently the only extra plumbing required is to >>>>> work-around the zero-argument constructor. Removing the setters would >>>>> mean more plumbing would be required for someone to create a custom JNDI >>>>> resource for a pool of objects of type X. >>>> >>>> But AFAIK JNDI does not allow dynamic changes, so can operate on the >>>> config classes - which do have the required characteristics, AFAICT. >>> >>> Which is what I mean by "more plumbing". Given that DBCP users want to >>> change pool config on the fly, I imagine users of other pools will to. >> >> I was just observing that the mutability requirements don't come from JNDI. >> Whether JNDI updates the config class or a mutable pool makes little >> or no difference to the code required. > > It makes little difference to the calling code, but the non-mutable > attributes adds the requirement for a config class (plumbing) and > something to inject the config before first use since JNDI resources > don't have standard start/stop methods that the contain can call (more > plumbing). If the resource is a bean, none of this plumbing is required. > >>>>> Given that there is a requirement from DBCP users for dynamic changes to >>>>> the pool, I believe POOL2 needs to support mutable pool implementations. >>>> >>>> The more mutable fields there are, the more work to be done to ensure >>>> thread-safety, and the more checking/unit tests needed when updating >>>> the code. >>>> >>>> So - are there any settings that it does not make sense to mutate >>>> after creation? >>>> >>>> For example, LIFO? >>> May as well be mutable. >> >> Might create some interesting logic issues if it changes whilst a one >> thread is using the pool and then another thread uses the new value. > > Possibly. We'll have to see. I can't think of any right now given the > implementation. > >>>> jmxEnabled/jmxNamePrefix? >>> That can / should be fixed. >> >> Does that mean set at instantiation only, or don't allow any changes >> once the pool starts to be used? > > Instantiation, since it needs to constant so it can be managed via JMX.
Actually, that is done already. I'd got the names from the config class, and forgot to check if it was stored in the pool classes - they're not, they're used in the ctor only. Sorry for the noise, however there just might be other fields that should be set in the ctor. >> Similarly for any other non-mutable items - the values must be >> established before they are first used. >> >> Could do this by disabling the setter if the pool is active. >> >> Or check for actual first use in the getter, but much easier to >> control if the field is final (i.e. set at construction). > > Using final would be my preference. > >>>> testOnBorrow/testOnReturn/testWhileIdle? >>> Mutable. >>> >>>> blockWhenExhausted? >>> Mutable. >>> >>>> And are there settings which need to be constrained relative to other >>>> settings? >>> No. We don't do this currently and I don't see a need to start. There >>> are some combinations that don't make much sense but we don't enforce >>> anything. >> >> OK, makes it simpler. > > Indeed :) > >>>> Would it make sense to only allow changes via the config class? >>> No. The config class can be a short-cut but I don't think it is the only >>> way. Note that changes via a config class won't be atomic unless we make >>> some changes. >>>> This would simplify the classes by eliminating all the setters, and >>>> should make consistency checking easier. >>> I don't think we need consistency checking. >>> >>> My current thinking is: >>> - make all mutable attributes volatile >> >> Yes. >> >>> - methods take local copies of the attributes they require at the start >>> of the method to ensure consistency >> >> That will ensure consistency within a method. >> >> It's not yet clear if there are any independent methods that rely on >> consistency between them. > > I don't believe that is the case. > > There is some work to do to check that this is currently how all the > methods are implemented. Now all access is via getters, checking that > getters are called at most once per method should be relatively easy. > >> Also if non-private method A calls non-private method B and they both >> need the same field, there's a potential window where the value could >> change unless special precautions are taken. It would be very easy to >> accidentally break the rule of only fetching each item once within a >> method invocation. > > That needs careful checking. I have been thinking about how we might > refactor things to avoid this entirely but haven't found an acceptable > solution yet. If all private methods were static, they could only access fields via parameters; that might help somewhat to ensure fetch-once behaviour. Probably a bit much to convert all non-private methods into simple wrappers for static private methods, but it would solve the issue. > Mark > > --------------------------------------------------------------------- > 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
