Hey,
is this regarding the feature to return a Future of the send to fail if netty is not able to send something, or? Julian ________________________________ Von: Sebastian Rühl <sebastian.ruehl...@googlemail.com.INVALID> Gesendet: Freitag, 26. Oktober 2018 16:24:28 An: dev@plc4x.apache.org Betreff: Re: OPM / Connection Pool And btw: Regarding timeouts: https://netty.io/4.1/api/io/netty/handler/timeout/ReadTimeoutHandler.html <https://netty.io/4.1/api/io/netty/handler/timeout/ReadTimeoutHandler.html> https://netty.io/4.1/api/io/netty/handler/timeout/WriteTimeoutHandler.html <https://netty.io/4.1/api/io/netty/handler/timeout/WriteTimeoutHandler.html> As simple as inserting one line of code in the pipelines :) `channel.pipeline().addLast("readTimeoutHandler", new ReadTimeoutHandler <https://netty.io/4.1/api/io/netty/handler/timeout/ReadTimeoutHandler.html>(30);` > Am 26.10.2018 um 16:17 schrieb Sebastian Rühl > <sebastian.ruehl...@googlemail.com>: > > 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 >> <mailto: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 >> <mailto:christofer.d...@c-ware.de>> >> Gesendet: Freitag, 26. Oktober 2018 16:01:37 >> An: dev@plc4x.apache.org <mailto: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 >> <mailto: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 >> <mailto:sebastian.ruehl...@googlemail.com.INVALID>> >> Gesendet: Freitag, 26. Oktober 2018 13:23:55 >> An: dev@plc4x.apache.org <mailto: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 <mailto: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 >>> <https://github.com/apache/incubator-plc4x/pull/30> >> >> >> >
