You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by "geneross (via GitHub)" <gi...@apache.org> on 2024/01/24 08:39:30 UTC

[PR] fix: added timeouts for reading data/getting connection

geneross opened a new pull request, #1361:
URL: https://github.com/apache/plc4x/pull/1361

   (no comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: added timeouts for reading data/getting connection

Posted by "geneross (via GitHub)" <gi...@apache.org>.
geneross commented on PR #1361:
URL: https://github.com/apache/plc4x/pull/1361#issuecomment-1978588277

   Thanks for paying attention. I was desperate to get an answer.  Lost in permissions etc. 
   In general we've faced with case when `TriggeredScraperImpl `threads just silently dies partially because of network malfunction partially because of server bugs. 
   So `TriggeredScraperImpl` got 2 debug threads that watch on working threads and it static get connection method now re-throw unchecked `PlcRuntimeException` because otherwise it might re-throw checked exception that leads to working thread termination. 
   Second. `PlcReadRequest.builder` from `LeasedPlcConnection` now return class with execution timeout for innerPlcReadRequest.
   Third. `ConnectionContainer` set container value to null when it can't get connection from the manager while trying to invalidate cached connection. Otherwise it created new `LeasedConnection` with same broken connection.
   
   My apologies for brevity, I have to recall what I did 2 month ago. )) 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: added timeouts for reading data/getting connection

Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #1361:
URL: https://github.com/apache/plc4x/pull/1361#issuecomment-1938872727

   However a bit more explanations on what a PR is about would have been nice ;-)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: added timeouts for reading data/getting connection

Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #1361:
URL: https://github.com/apache/plc4x/pull/1361#issuecomment-1938872157

   LGTM


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: added timeouts for reading data/getting connection

Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on code in PR #1361:
URL: https://github.com/apache/plc4x/pull/1361#discussion_r1512799098


##########
plc4j/tools/connection-cache/src/main/java/org/apache/plc4x/java/utils/cache/LeasedPlcConnection.java:
##########
@@ -120,14 +126,18 @@ public PlcReadRequest build() {
                 return new PlcReadRequest(){
                     @Override
                     public CompletableFuture<? extends PlcReadResponse> execute() {
-                        CompletableFuture<? extends PlcReadResponse> future = innerPlcReadRequest.execute();
+                        CompletableFuture<? extends PlcReadResponse> future =
+                            innerPlcReadRequest.execute().orTimeout(Math.min(1000,maxUseDuration.toMillis()), TimeUnit.MILLISECONDS);

Review Comment:
   So as far as I remember the ConnectionPool ... that used to have a timer to invalidate a connection if being used too long ... but I guess in that case possibly the read operation never timed out, right? So this is an additional abort on the side of the user, correct?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: added timeouts for reading data/getting connection

Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz merged PR #1361:
URL: https://github.com/apache/plc4x/pull/1361


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org