On Thursday, February 6, 2014, Nick Williams <nicho...@nicholaswilliams.net> wrote:
> > On Feb 5, 2014, at 5:52 PM, Remko Popma wrote: > > > > On Thursday, February 6, 2014, Nick Williams < > nicho...@nicholaswilliams.net<javascript:_e(%7B%7D,'cvml','nicho...@nicholaswilliams.net');>> > wrote: > >> Guys, >> >> Currently, the JDBCAppender allows users to specify a mechanism for >> connecting to the database using one of three options: >> >> - DataSourceConnectionSource: Looks up a JNDI data source >> - FactoryMethodConnectionSource: User specifies a class and static method >> for retrieving connections >> - DriverManagerConnectionSource: User specifies JDBC URL, username, >> password, etc. to manually connect directly from Log4j. >> >> Here's the problem: connections really need to be pooled for Log4j to log >> efficiently. In fact, I'd go so far as to say it's a *requirement*. It will >> either be flaky (if using the same connection continuously) or horrendously >> slow (if reconnecting every time) without pooling. >> >> DataSourceConnectionSource and FactoryMethodConnectionSource lend >> themselves naturally to pooling. We can simply tell the user, 1) the >> DataSource must be a pooled DataSource or performance will suffer greatly, >> and 2) The factory must be backed by a connection pool or performance will >> suffer greatly. At that point, it's out of our hands and left up to the >> user to pool it. I like this. >> >> DriverManagerConnectionSource is a different story. Since Log4j connects >> directly using this approach, we're left with two options: >> >> - Remove support for DriverManagerConnectionSource and force the user to >> supply a factory or DataSource. (This is my favorite option.) > > > I'd be fine with this option. If our experience so far shows that we can't > really support direct connections (that is, we don't have good solutions > for user problems) then removing it is not unreasonable I think. > > >> - Add Commons DBCP and Commons Pooling as required dependencies when >> using DriverManagerConnectionSource, then update >> DriverManagerConnectionSource to also accept connection pool size, >> thresholds, maximums, minimums, test queries, etc. (I really, *REALLY* >> don't like this option.) > > > If Commons DBCP can be set up as a pooled DataSource, then there is no > need for us to also support its use as a DriverManagerConnectionSource, > is there? > > >> I'm looking for some input from the rest of y'all on which direction we >> should take, or if you can think of any other options. > > > > One other option (or perhaps this is more a migration path issue) is to > leave the DriverManagerConnectionSource there but mark it as > deprecated/dangerous/broken. Emit an ominous warn status logger message for > existing users, and in the docs clarify that this will be removed in a > subsequent release because we can't support it. > > > It would be rather illogical to deprecate a class and say it will be > remove "later" when we haven't even gone GA yet. > Some people are using beta-9 in production, and I was thinking to provide a migration path for them. But on second thought, simply removing is cleaner. If people want to upgrade they should use a pooled connection source. I'm fine with that. > Nick >
