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
> 
> 
> 

Reply via email to