Hi Christopher, I'm sorry for the delay. I've finally found some time to think about your E-Mail. Comments inline. 2013/3/11 Christopher Deckers <[email protected]>
> Hi Lukas, > > I see two main categories of ExecuteListeners: > - Stateless ExecuteListener. > - Stateful ExecuteListener, for example to collect statistics. > > My problem is with stateful ExecuteListener. > > In 2.x, we were registering ExecuteListener classes, which acted like a > factory of ExecuteListener instances, and a new instance was created for > each query execution. This allowed to declare states directly in the > ExecuteListener class. In case we wanted central collection of statistics > over time (about all query executions), we could use a statistic cache of > some sort. > > In 3.0 RC2, we register a listener instance instead of a factory. This is > not a problem for stateless ExecuteListener, but it becomes problematic for > stateful ExecuteListener. While it simplifies cases like central collection > over time, it complexifies keeping states local to a single query > execution. Indeed, many threads could concurrently invoke the > ExecuteListener, and even for a single thread it seems there are cases > where a new execution can take place before the previous execution > completes (e.g.: fetchLazy() results in end() being called late). > > . Thus, how can we store (and retrieve in each event) a state object that > only lives for the time of a query execution? > That is a good question. The current API doesn't help you much > It seems that the ExecuteContext instance is per query execution so > get/setData sounds like a logical fit. But it seems that the actual data is > potentially not stored in the ExecuteContext itself and could be shared and > as such cannot be used to solve this problem. If the data were stored in > the context, then we would not have to perfom any cleanup. > Right. Configuration's getData() and setData() methods share the Configuration's lifecycle, i.e. they live as long as the original Executor. The problem here is the fact that ExecuteContext extends Configuration, rather than referencing it. Maybe, composition should be chosen over inheritance, here. This would resolve confusion about the responsibility and lifecycle of the involved objects. Solving this problem should probably also introduce an additional Map that can be used to store objects in the context, for the lifetime of a single execution. These changes should still make it into jOOQ 3.0: https://github.com/jOOQ/jOOQ/issues/2343 > The other possibility/workaround would be to retain the state in an > identity map in the ExecuteListener which key is the context (stored in > start() and removed in end()). What I don't like is that we have to make > sure that end() is called in every case (e.g.: interruption of fetchLazy, > etc) so that we can perform state cleanup, or worse, implement the cache > with soft references and a thread that occasionaly cleans dead references. > That would work. The ExecuteContext instance is the right reference to use as a key in an "identity map", as its lifecycle matches that of a full query execution. You can rely on end() being called, to clean up your state. If end() isn't called, then it's a bug. > On a related note, I don't see why ExecuteContext.getData() restricts keys > to Strings. It could as well be Object, with a contract on whether > hashcode/equals is used or something else. > You're right. Other key types should be allowed, too. Registered as #2342 https://github.com/jOOQ/jOOQ/issues/2342 Thanks again for sharing your thoughts. As you didn't get any reply from others, I take that there are no objections to your reasoning... Of course, not everyone challenges the jOOQ API as much as you do :-) 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.
