Christopher, Finally I get to answering this E-Mail. It's been a busy week!
A couple of definitions to keep up to date with the latest changes - DSL: The new name of what was called "Factory" before - DSLContext: The new name of what was called "Executor" before - DSL.using(...), returning DSLContext: The new way of doing what could be done through "new Executor(...)" before (with the same overloading) This is the latest state as available on Github master and on the Sonatype SNAPSHOT repository for 3.0-RC3. Note, that using() was chosen rather than with(), to avoid issues with Scala/Groovy reserved words. See comments by Ben Manes here: https://github.com/jOOQ/jOOQ/issues/2379 Now, comments inline: 2013/3/30 Christopher Deckers <[email protected]>: > Hi Lukas, > > OK, here is the API challenge of the day. > This is to sum up my views on Executor and Configuration and hopefully also > make users of the library react! :) Hehe, no one seemed to have reacted to this. :-) It's a tough and complex topic! Unlike with the other topics in that area, this one is harder to get "right" - whatever "right" is. > I believe there are several cases for Executor creation: > - Create an Executor with ad-hoc parameters. > - Create an Executor with a Configuration. > - And eventually: create an Executor with a Configuration, overridden with > ad-hoc parameters. > > I am mainly focusing on the 2nd/3rd cases: creating an Executor with a > Configuration. OK, I agree with that. > When someone uses a Configuration, I believe that the purpose is to not > repeat parameters, to have the same parameters for all Executors. In some > cases, which I think are limited, someone could have more than one > Configuration, but would still use those as shared parameters for sets of > Executors. The purpose of this e-mail is to suggest strengthening this > notion of Configuration as a global shareable set of parameters. Yes, I agree with that, too. While it is a good use-case to create ad-hoc configurations through what you called "ad-hoc parameters", in the long run, users will reuse mostly a single, global Configuration. > In my view, because a Configuration shares parameters, it should by nature > not share object instances that have a state that depends on execution. > Otherwise, 2 calls from different threads could fail if they happen at the > same time; 2 sequencial queries where the first one lazily fetches the > results could also fail. > The best a Configuration should do in this area is to provide a factory of > such execution-dependant objects. Actual instances should be set on the > Executor directly. I wish to exclude the DSLContext / Executor from these thoughts. It is complicating things unnecessarily. It has no state / lifecycle of its own. Introducing it in the way I originally did was confusing. The thought of having "new Executor" along with the name "Executor" seemed to indicate that it has a life of its own. It doesn't. It had a 1-to-1 relationship with Configuration, adding DSL capabilities. As I will explain further down, everything you want to do can be done without allowing "Executor" to have its own lifecycle. There are only these types of lifecycle in jOOQ: - The Configuration's. It is allowed to be global, but if desired, it can also be "local", i.e. execution-based. Let's assume it is global. - The ExecuteContext's. It is defined to be "local", i.e. execution-based. An ExcuteContext will go through at least the ExecuteListener.start() and ExecuteListener.end() events. For that, it may (but is not required to) go through ConnectionProvider.acquire() and ConnectionProvider.release() events. An ExecuteContext is always understood in the context of the Configuration that created it. - The RenderContext's and BindContext's. These are irrelevant for this discussion. Let's reiterate the above, looking at the most up-to-date API: // Wrap a Configuration in a DSLContext: DSLContext ctx = using(configuration); // ctx2 is no different from ctx1. It is just a DSL-object. DSLContext ctx2 = using(configuration); // Create a local ExecuteContext from the above Configuration ctx.fetch("SELECT 1 FROM DUAL"); // Create another local ExecuteContext from the above Configuration ctx.fetch("SELECT 2 FROM DUAL"); > The main objects that may have a state which depends on execution currently > are: > - ExecuteListener: an actual instance is plugged to the Configuration > object. Storing state directly in that class is very dangerous and the > workaround is to use the data map of the ExecuteContext to store and > retrieve execution-related state. Failing to do so may work until 2 threads > call it at the same time, etc. > - ConnectionProvider: the specification is very loose on how to implement > it, which means that the same connection could be acquired by 2 threads > using the same configuration. I agree. Although, I think that the ConnectionProvider *should* be loose to cover all use cases. > Because of the current structure of the API, it is easy to misuse and to > share a configuration object which eventually shares stateful execute > listeners and connections (through connection providers). It would > eventually blow up randomly depending on threading or query execution flows > (preferably once deployed in production) and would of course be very hard to > debug. Yes, it is hard to get it right. Do note that this was much harder to get right before jOOQ 3.0, before the introduction of a ConnectionProvider. In order to get it right, a properly pooled (and thus shareable) DataSource was needed. Even now, looking at rather elaborate integrations shows how hard things can get: https://gist.github.com/ben-manes/5316351 While ConnectionProvider was easy to implement in this case > I think ExecuteListener and ConnectionProvider should be replaced so that > the user can: > - Set an actual instance of X (see below) on an Executor instance. > - Set a factory of X in the Configuration, which can be used by many > Executor instances. > > > About ExecuteListener, the user could either: > > 1. Define an ad-hoc ExecuteListener instance: > Executor executor = new Executor(configuration); > executor.getExecuteListeners().add(new MyExecuteListener()); > > 2. Define a global ExecuteListener factory: > configuration = new Configuration(); > configuration.getExecuteListenerFactories().add(new ExecuteListenerFactory() > { > public ExecuteListener createExecuteListener() { > return new MyExecuteListener(); > } > }); > // Everywhere the configuration is used, then: > Executor executor = new Executor(configuration); > > A few notes: > - If per-execution state is needed, the factory would just create a new > instance, which is the general case. Of course, the factory can supply the > same instance of the ExecuteListener if it wishes so, but then it is not a > factory anymore and as such the implementor will have to create its > ExecuteListener subclass with thread safety in mind. Such cases are for > example for aggregation of statistics over multiple queries and threads, and > is by nature required to be coded in a thread safe way. > - If using a factory seems like clutter, keep in mind that the configuration > is to define shared parameters and is not done at many places in the code. > Moreover, with Lambda in Java 8 this turns into a one liner. Yes, here the configuration should hold a sort of ExecuteListenerFactory (actual name may differ), rather than actual ExecuteListener state. Where I don't agree is the distinction between 1. and 2. for two reasons: A) Users can already create new Configurations and set them on per-execution configured DSLContexts B) jOOQ can provide default implementations for the ExecuteListenerFactory, e.g. a DefaultExecuteListenerFactory, which would contain a constant list of ExecuteListeners. This default behaviour would model your case number 1., or today's behaviour in jOOQ. > About the ConnectionProvider, the user could either: > > 1. Define an ad-hoc connection instance: > Executor executor = new Executor(conn, sqlDialect); > > 2. Define a global factory: > configuration = new Configuration(); > configuration.setConnectionProvider(new MyPoolBasedConnectionProvider()); > // Everywhere the configuration is used, then: > Executor executor = new Executor(configuration); > > A few notes: > - The ConnectionProvider would not be loose in its meaning as it is > currently and it would specify that every connection that is obtained > through acquire() cannot be returned twice until it is returned with > release(conn). This way, the connection provider can be used by concurrent > queries. > - A side effect, but not the main motivation, is that the ConnectionProvider > would be a safe place to acquire/release dedicated connections. 3rd party > ExecuteListener or whatever tools could make use of such capability. > - Maybe we should also have signatures like: "new Executor(Configuration, > conn)". The general idea would be that the configuration is used, but the > executor can override some aspects of it like what I did with the > ExecuteListener above. In fact, maybe we should have an API so that user can > call "new Executor(Configuration)" and then call > "executor.setConnection(conn)" if so they wish. > - Because there is this idea that the Configuration is shared, when an > Executor is instanciated it would create an internal copy to merge > overridden aspects (connection, execute listeners, even rendering > capabilities that could be accessed through executor.getConfiguration()). Again, I don't think that we need the additional complexity by adding the possibility of creating "per-Executor" setups. Users that need such a setup can already A) Create new Configurations and set them on per-execution configured DSLContexts B) Use jOOQ's existing default implementations, e.g. DefaultConnectionProvider, by calling the following: // Create an ad-hoc Configuration using(connection, dialect); > I strongly believe that it would make the API easier to understand (global > vs ad-hoc) and would reduce the chances of misusing it. Please let me know > if my thoughts are completely off-track or if such API would be beneficial > to jOOQ. No, you're completely on-track. The actual solution might in fact be a bit simpler, as you described above. The main thing here is that there shouldn't be a List<ExecuteListener> in Configuration. Instead, there should be an ExecuteListenerProvider, as an analogue to ConnectionProvider. I think its contract should be loose as well, though. I feel that setting strong contracts on these Provider types might: 1. Make things even harder to implement correctly 2. Not solve the actual problem, which is ExecuteListener state in the Configuration I have registered #2388 for the above. This will go into 3.0-RC3: https://github.com/jOOQ/jOOQ/issues/2388 If you feel that there is still a missing piece as I might not have gotten the full picture yet (e.g. contracts too loose, additional lifecycle for DSLContext), feel free to continue this discussion. Thanks for your feedback! Cheers Lukas -- You received this message because you are subscribed to the Google Groups "jOOQ User Group" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
