Hey Sebastian,
the only problem witht he reuse i had is possible internal state during in-flight messages or problems with multi threading (I'm not sure if all drivers are thread safe and stateless during in-flight messages), but I agree that this is minor currently, just wanted to add the todo to keep it in view : ) Request Limiting is, as I think, no concern of the driver itself, I think, so I would add request limiting probably in an layer above thus I'm totally fine. With regards to Marcels branch I thought it would be ideal to move over its ProxyConnection as this is everything thats needed to fix the todo (all methods delegate untill close() is called then all throw an unsupported operation exception. This is similar to what common jdbc pools do, I checked dbcp and some other one). Yes, we can directly add the validation hook from commons pool, but I think we need a plc specific ping method in the api to call as we have no "common" field or "query" we can issue. Best Julian ________________________________ Von: Sebastian Rühl <sebastian.ruehl...@googlemail.com.INVALID> Gesendet: Freitag, 26. Oktober 2018 13:23:55 An: dev@plc4x.apache.org Betreff: Re: OPM / Connection Pool Hi Julian, Regarding the reuse of Connections: I was aware of that issue but forgot to add a TODO for that (sorry for that). But in general a pool is not a request limiter rater a connection limiter. Therefore the continue use shouldn’t be that big of an issue for the time being (Till we resolve that issue). Request Limiting: A separate generic request limiter feature would be nice but is out-of-scope of the pool. In case of S7 this is builtin (maxAmqCaller/maxAmqCallee). Regarding the branch of Marcel (And thanks for the great contribution Marcel :): He did some good work but at the end the implementation was initially too different from the commons-pool one as I would have reverted/changed almost every line. So the idea was it to write it upfront in the separate module to have the possibility to compare it side by side and pick the goodies. Regarding validation: The commons-pool validation calls isConnected() on return. So we could implement the „ping“ in the isConnected() as well or implement a ping() as you described. Sebastian > Am 25.10.2018 um 23:13 schrieb Julian Feinauer <j.feina...@pragmaticminds.de>: > > Hey all, > > just wanted to keep you up to date with some things Sebastian and I discussed > off-list. > Thanks to Sebastians support the Object Plc Mapping (opm) is now mergend into > master and I would love to get some feedback. > I hope that I can soon add some documentation on the side about that and I > would be happy if someone likes to implement writing (currently only reading > is supported), see PLC4X-70. > > Sebastian also added an implementation for the Connection Pool. This can > become one of the most important features to make it usable in enterprise > applications (pooling, keep connections open, …). > It is important to note that Marcel opened a PR for the same feature and my > impression is that both implementations are lacking some (different) things > and thus, together, are a pretty good first shot. > Thus, I suggest that we keep PR-30 open [1] or at least the branch intact and > move the Proxy Implementations over (added a TODO in the > PooledPlcDriverManager which is dangerous, I think) and also add some > validation (this should be added to the driver interface or the SPI > somewhere, I guess to be able to have a reasonable “ping” for all plc’s, it > is in our case not as easy as a “SELECT 1” in JDBC). > > Thus, we are on a great way, and as soon as we have moved over the necessary > parts from PR-30 and probably also added the validation I would be very happy > to ship OPM as feature in one of our applications. I’ll then also prepare a > sample on that because it makes PLC Connections really as easy as querying a > Database via JPA. > > Thanks especially to Marcel and Sebastian! > Julian > > [1] https://github.com/apache/incubator-plc4x/pull/30
