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
