You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by Christofer Dutz <ch...@c-ware.de> on 2023/03/17 06:56:12 UTC

Update to the connection-cache

Hi all,

I think I‘ve finally managed to spare some time to finish my work on the connection cache.

I think https://github.com/apache/plc4x/pull/818 tried to address a problem, where if something happens with the connection while it’s being used, that it might be in a inconsistent state. So, it was the goal to intercept any errors occurring during the execution of any request and to instantly close the connection if anything went wrong.

However, I wasn’t happy with the PR as it sort of brought the connection into an odd state and it only was a partial solution. Also did it change the API, weakening the contract between PLC4X and the using application. And if there were requests waiting for the connection, they would just have been dropped and would never get a connection.

So, what I did, was something similar to the original PR: I intercept the execution of all types of requests (not only read- and write-requests). But instead of closing the connection instantly, I mark it as “invalidateConnection” … when the close() method is called, this tells the connection container to invalidate the connection.

The connection container then handles closing the connection and possibly creating a new connection and using that to serve the waiting clients.

I hope this solves the problem people were seeing with the current implementation.

I will then close the PR818 if I don’t hear anything in the next 72h

Chris



AW: Update to the connection-cache

Posted by Christofer Dutz <ch...@c-ware.de>.
Hi Youlin,

Well, it not being complete was only one part … so the reason you needed to change the API in order to get it working was due to the fact that you were closing the connection on error instantly. My approach just marks the connection as broken and closes it when it’s supposed to be closed. This way also the other problems that made you need the API change.

My fix is finished and in develop … please check if this doesn’t also fix your problems … in that case I would close the PR (Or you do it)

The timeout handling is a separate issue for me.

Chris



Von: youlin he <he...@gmail.com>
Datum: Freitag, 17. März 2023 um 12:02
An: dev@plc4x.apache.org <de...@plc4x.apache.org>
Betreff: Re: Update to the connection-cache
I'm just pointing out the problem with the connection pool about the PR 818. My current test environment only has read and write.
If you think this PR is valuable, I will take the time to continue to improve.

Christofer Dutz <ch...@c-ware.de>> 于2023年3月17日周五 14:56写道:
Hi all,

I think I‘ve finally managed to spare some time to finish my work on the connection cache.

I think https://github.com/apache/plc4x/pull/818 tried to address a problem, where if something happens with the connection while it’s being used, that it might be in a inconsistent state. So, it was the goal to intercept any errors occurring during the execution of any request and to instantly close the connection if anything went wrong.

However, I wasn’t happy with the PR as it sort of brought the connection into an odd state and it only was a partial solution. Also did it change the API, weakening the contract between PLC4X and the using application. And if there were requests waiting for the connection, they would just have been dropped and would never get a connection.

So, what I did, was something similar to the original PR: I intercept the execution of all types of requests (not only read- and write-requests). But instead of closing the connection instantly, I mark it as “invalidateConnection” … when the close() method is called, this tells the connection container to invalidate the connection.

The connection container then handles closing the connection and possibly creating a new connection and using that to serve the waiting clients.

I hope this solves the problem people were seeing with the current implementation.

I will then close the PR818 if I don’t hear anything in the next 72h

Chris


Re: Update to the connection-cache

Posted by youlin he <he...@gmail.com>.
I'm just pointing out the problem with the connection pool about the PR
818. My current test environment only has read and write.
If you think this PR is valuable, I will take the time to continue to
improve.

Christofer Dutz <ch...@c-ware.de> 于2023年3月17日周五 14:56写道:

> Hi all,
>
> I think I‘ve finally managed to spare some time to finish my work on the
> connection cache.
>
> I think https://github.com/apache/plc4x/pull/818 tried to address a
> problem, where if something happens with the connection while it’s being
> used, that it might be in a inconsistent state. So, it was the goal to
> intercept any errors occurring during the execution of any request and to
> instantly close the connection if anything went wrong.
>
> However, I wasn’t happy with the PR as it sort of brought the connection
> into an odd state and it only was a partial solution. Also did it change
> the API, weakening the contract between PLC4X and the using application.
> And if there were requests waiting for the connection, they would just have
> been dropped and would never get a connection.
>
> So, what I did, was something similar to the original PR: I intercept the
> execution of all types of requests (not only read- and write-requests). But
> instead of closing the connection instantly, I mark it as
> “invalidateConnection” … when the close() method is called, this tells the
> connection container to invalidate the connection.
>
> The connection container then handles closing the connection and possibly
> creating a new connection and using that to serve the waiting clients.
>
> I hope this solves the problem people were seeing with the current
> implementation.
>
> I will then close the PR818 if I don’t hear anything in the next 72h
>
> Chris
>
>
>