You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2020/10/27 13:35:09 UTC

[GitHub] [qpid-dispatch] ganeshmurthy opened a new pull request #897: DISPATCH-1815: Send PING frame every 4 seconds on an egress connection

ganeshmurthy opened a new pull request #897:
URL: https://github.com/apache/qpid-dispatch/pull/897


   


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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] ganeshmurthy closed pull request #897: DISPATCH-1815: Send PING frame every 4 seconds on an egress connection

Posted by GitBox <gi...@apache.org>.
ganeshmurthy closed pull request #897:
URL: https://github.com/apache/qpid-dispatch/pull/897


   


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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] grs commented on a change in pull request #897: DISPATCH-1815: Send PING frame every 4 seconds on an egress connection

Posted by GitBox <gi...@apache.org>.
grs commented on a change in pull request #897:
URL: https://github.com/apache/qpid-dispatch/pull/897#discussion_r512855336



##########
File path: src/adaptors/http2/http2_adaptor.h
##########
@@ -138,6 +141,7 @@ struct qdr_http2_connection_t {
     bool                      ingress;
     bool                      timer_scheduled;
     bool                      client_magic_sent;
+    time_t                    prev_ping; // Time the previous PING frame was sent on egress connection.

Review comment:
       Right, and that is fine. All I'm saying is that if you send out any other frame within the second, there is no need to send a ping. However that is an optimisation. It is good as is.




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] ganeshmurthy commented on pull request #897: DISPATCH-1815: Send PING frame every 4 seconds on an egress connection

Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on pull request #897:
URL: https://github.com/apache/qpid-dispatch/pull/897#issuecomment-717248525


   @grs please take a look and comment and/or approve


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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] ganeshmurthy commented on a change in pull request #897: DISPATCH-1815: Send PING frame every 4 seconds on an egress connection

Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #897:
URL: https://github.com/apache/qpid-dispatch/pull/897#discussion_r512861474



##########
File path: src/adaptors/http2/http2_adaptor.h
##########
@@ -138,6 +141,7 @@ struct qdr_http2_connection_t {
     bool                      ingress;
     bool                      timer_scheduled;
     bool                      client_magic_sent;
+    time_t                    prev_ping; // Time the previous PING frame was sent on egress connection.

Review comment:
       Ah, yes, agreed. Good point. 




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] grs commented on a change in pull request #897: DISPATCH-1815: Send PING frame every 4 seconds on an egress connection

Posted by GitBox <gi...@apache.org>.
grs commented on a change in pull request #897:
URL: https://github.com/apache/qpid-dispatch/pull/897#discussion_r512725180



##########
File path: src/adaptors/http2/http2_adaptor.h
##########
@@ -138,6 +141,7 @@ struct qdr_http2_connection_t {
     bool                      ingress;
     bool                      timer_scheduled;
     bool                      client_magic_sent;
+    time_t                    prev_ping; // Time the previous PING frame was sent on egress connection.

Review comment:
       Strictly this could be the last time for any write to the socket, not just a ping. The time could then be updated on every write.




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] ganeshmurthy commented on a change in pull request #897: DISPATCH-1815: Send PING frame every 4 seconds on an egress connection

Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #897:
URL: https://github.com/apache/qpid-dispatch/pull/897#discussion_r512825599



##########
File path: src/adaptors/http2/http2_adaptor.h
##########
@@ -138,6 +141,7 @@ struct qdr_http2_connection_t {
     bool                      ingress;
     bool                      timer_scheduled;
     bool                      client_magic_sent;
+    time_t                    prev_ping; // Time the previous PING frame was sent on egress connection.

Review comment:
       The check_send_ping_frame() only conditionally updates the conn->prev_ping field like this - 
   
   
   
       time_t current = time(NULL);
       time_t prev = conn->prev_ping;
       if (current - prev >= 4) {
           send_ping_frame(conn);
           qd_log(http2_adaptor->log_source, QD_LOG_TRACE, "[C%"PRIu64"] Sent PING frame", conn->conn_id);
           qd_timer_schedule(conn->ping_timer, 4000);
           conn->prev_ping = current;
       }
   
   So the conn->prev_ping contains the time that the ping was sent out ?




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org