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/02/19 18:52:52 UTC
[GitHub] [qpid-dispatch] kgiusti opened a new pull request #689:
DISPATCH-1576: Avoid Q2/Q3 flow control on router control links
kgiusti opened a new pull request #689: DISPATCH-1576: Avoid Q2/Q3 flow control on router control links
URL: https://github.com/apache/qpid-dispatch/pull/689
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] kgiusti closed pull request #689: DISPATCH-1576:
Avoid Q2/Q3 flow control on router control links
Posted by GitBox <gi...@apache.org>.
kgiusti closed pull request #689: DISPATCH-1576: Avoid Q2/Q3 flow control on router control links
URL: https://github.com/apache/qpid-dispatch/pull/689
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] kgiusti commented on issue #689: DISPATCH-1576:
Avoid Q2/Q3 flow control on router control links
Posted by GitBox <gi...@apache.org>.
kgiusti commented on issue #689: DISPATCH-1576: Avoid Q2/Q3 flow control on router control links
URL: https://github.com/apache/qpid-dispatch/pull/689#issuecomment-591051013
Abandoned: need to keep backpressure on inter-router links
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#689: DISPATCH-1576: Avoid Q2/Q3 flow control on router control links
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #689: DISPATCH-1576: Avoid Q2/Q3 flow control on router control links
URL: https://github.com/apache/qpid-dispatch/pull/689#discussion_r381641296
##########
File path: src/router_node.c
##########
@@ -1415,16 +1419,18 @@ static void CORE_link_second_attach(void *context, qdr_link_t *link, qdr_terminu
//
pn_link_open(pn_link);
- qd_connection_t *conn = qd_link_connection(qlink);
- qdr_connection_t *qdr_conn = (qdr_connection_t*) qd_connection_get_context(conn);
+ // DISPATCH-1460, DISPATCH-1576 Do not block router control messages.
+ // Control messages must be received in their entirety before they are
+ // consumed. Flow controlling such messages mean they will never fully
+ // arrive thus never be consumed (therefore flow control is never
+ // released).
//
- // All links on the inter router or edge connection have unbounded q2 limit
- //
- if (qdr_connection_role(qdr_conn) == QDR_ROLE_EDGE_CONNECTION || qdr_connection_role(qdr_conn) == QDR_ROLE_INTER_ROUTER) {
+ const qd_link_type_t lt = qdr_link_type(link);
+ if (lt == QD_LINK_CONTROL || lt == QD_LINK_EDGE_DOWNLINK) {
Review comment:
I don't think we need to include QD_LINK_EDGE_DOWNLINK. Links of QD_LINK_EDGE_DOWNLINK must be backpressured just like the other links. The only link that should not be front pressured or back pressured is QD_LINK_EDGE_DOWNLINK
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] kgiusti commented on a change in pull request #689:
DISPATCH-1576: Avoid Q2/Q3 flow control on router control links
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #689: DISPATCH-1576: Avoid Q2/Q3 flow control on router control links
URL: https://github.com/apache/qpid-dispatch/pull/689#discussion_r381479334
##########
File path: src/router_node.c
##########
@@ -1381,12 +1381,16 @@ static void CORE_link_first_attach(void *context,
//
pn_link_open(qd_link_pn(qlink));
+ // DISPATCH-1460, DISPATCH-1576 Do not block router control messages.
+ // Control messages must be received in their entirety before they are
+ // consumed. Flow controlling such messages mean they will never fully
+ // arrive thus never be consumed (therefore flow control is never
+ // released).
//
- // All links on the inter router or edge connection have unbounded q2 limit.
- // Blocking control messages can lead to various failures
- //
- if (qdr_connection_role(conn) == QDR_ROLE_EDGE_CONNECTION || qdr_connection_role(conn) == QDR_ROLE_INTER_ROUTER) {
+ const qd_link_type_t lt = qdr_link_type(link);
+ if (lt == QD_LINK_CONTROL || lt == QD_LINK_EDGE_DOWNLINK) {
Review comment:
This ignores a class of locally terminated link: the core endpoint links established by the core. Perhaps those types of links should be QD_LINK_CONTROL rather than ENDPOINT?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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
#689: DISPATCH-1576: Avoid Q2/Q3 flow control on router control links
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #689: DISPATCH-1576: Avoid Q2/Q3 flow control on router control links
URL: https://github.com/apache/qpid-dispatch/pull/689#discussion_r381641296
##########
File path: src/router_node.c
##########
@@ -1415,16 +1419,18 @@ static void CORE_link_second_attach(void *context, qdr_link_t *link, qdr_terminu
//
pn_link_open(pn_link);
- qd_connection_t *conn = qd_link_connection(qlink);
- qdr_connection_t *qdr_conn = (qdr_connection_t*) qd_connection_get_context(conn);
+ // DISPATCH-1460, DISPATCH-1576 Do not block router control messages.
+ // Control messages must be received in their entirety before they are
+ // consumed. Flow controlling such messages mean they will never fully
+ // arrive thus never be consumed (therefore flow control is never
+ // released).
//
- // All links on the inter router or edge connection have unbounded q2 limit
- //
- if (qdr_connection_role(qdr_conn) == QDR_ROLE_EDGE_CONNECTION || qdr_connection_role(qdr_conn) == QDR_ROLE_INTER_ROUTER) {
+ const qd_link_type_t lt = qdr_link_type(link);
+ if (lt == QD_LINK_CONTROL || lt == QD_LINK_EDGE_DOWNLINK) {
Review comment:
I don't think we need to include QD_LINK_EDGE_DOWNLINK. Links of QD_LINK_EDGE_DOWNLINK must be backpressured just like the other links. The only link that should not be front pressured or back pressured is QD_LINK_CONTROL
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org