Hi Both, What we would need then is something like this: https://netty.io/4.1/api/io/netty/handler/traffic/ChannelTrafficShapingHandler.html <https://netty.io/4.1/api/io/netty/handler/traffic/ChannelTrafficShapingHandler.html>
…only message based (e.g. 100 requests). This could then be implemented in driver-bases (like the SingleItemToSingleRequestProtocol) and could be used for protocol that don’t have sophisticated traffic control implementation like S7 Sebastian > Am 26.10.2018 um 16:04 schrieb Julian Feinauer <j.feina...@pragmaticminds.de>: > > Hey Chris, > > > yes, indeed, sorry for the confusion. > > I agree with this "low level" limiting, as this is simply necessary to ensure > connection. > > What I mean is that we could decide in an application to not send more than > 100 requests / s (for whatever reason) and handle that before forwarding the > requests to the pool. > > > Julian > > ________________________________ > Von: Christofer Dutz <christofer.d...@c-ware.de> > Gesendet: Freitag, 26. Oktober 2018 16:01:37 > An: dev@plc4x.apache.org > Betreff: Re: OPM / Connection Pool > > Hi Julian, > > I think we might be thinking of different things when referring to "limiting". > In my case I know that a Siemens S7 device is able to process a maximum > number of concurrent requests. If more are sent, they are simply discarded > and an error response is sent. > This maximum number of concurrent requests is agreed upon during the > connection phase in the maxAmqCaller and maxAmqCallee parameters. The S7 > driver now automatically limits the number of requests sent out to respect > these boundaries. And I don't see an option to handle this generically except > to limit it to 1 globally. > > I think you are referring to something else. Could you please explain to us > what you mean with "request limiting". > > Chris > > Am 26.10.18, 15:55 schrieb "Julian Feinauer" <j.feina...@pragmaticminds.de>: > > 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 > > >