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 2021/02/11 00:10:55 UTC
[GitHub] [qpid-dispatch] kgiusti opened a new pull request #1027: checkpoint
kgiusti opened a new pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027
This generalizes the Q2 receiver restart logic by replacing the harcoded calls to qd_link_restart_rx and the qd_link_t_sp in the message handling with a generic callback handler suitable for use with the adaptors.
----------------------------------------------------------------
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 #1027: WIP: generic Q2 unblocking API
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027#discussion_r574598410
##########
File path: include/qpid/dispatch/alloc_pool.h
##########
@@ -80,7 +80,26 @@ void *qd_alloc(qd_alloc_type_desc_t *desc, qd_alloc_pool_t **tpool);
/** De-allocate from a thread pool. Use via ALLOC_DECLARE */
void qd_dealloc(qd_alloc_type_desc_t *desc, qd_alloc_pool_t **tpool, char *p);
uint32_t qd_alloc_sequence(void *p);
-static inline void qd_nullify_safe_ptr(qd_alloc_safe_ptr_t *sp) { sp->ptr = 0; }
+
+// generic safe pointer api for any alloc pool item
+
+#define QD_SAFE_PTR_INIT(p) { .ptr = (void*)(p), .seq = qd_alloc_sequence(p) }
+
+static inline void qd_nullify_safe_ptr(qd_alloc_safe_ptr_t *sp)
Review comment:
Instead of this function (qd_nullify_safe_ptr), can we just pass in a zero to qd_alloc_set_safe_ptr(). That way when people search for qd_alloc_set_safe_ptr, we can see where the safe pointer is set and unset. I do understand that this PR is not introducing the qd_nullify_safe_ptr() function.
----------------------------------------------------------------
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] kgiusti commented on a change in pull request #1027: DISPATCH-1960: protocol generic Q2 unblocking API
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027#discussion_r575484508
##########
File path: include/qpid/dispatch/alloc_pool.h
##########
@@ -80,7 +80,26 @@ void *qd_alloc(qd_alloc_type_desc_t *desc, qd_alloc_pool_t **tpool);
/** De-allocate from a thread pool. Use via ALLOC_DECLARE */
void qd_dealloc(qd_alloc_type_desc_t *desc, qd_alloc_pool_t **tpool, char *p);
uint32_t qd_alloc_sequence(void *p);
-static inline void qd_nullify_safe_ptr(qd_alloc_safe_ptr_t *sp) { sp->ptr = 0; }
+
+// generic safe pointer api for any alloc pool item
+
+#define QD_SAFE_PTR_INIT(p) { .ptr = (void*)(p), .seq = qd_alloc_sequence(p) }
+
+static inline void qd_nullify_safe_ptr(qd_alloc_safe_ptr_t *sp)
Review comment:
I think qd_nullify.. is used in other places outside of Q2. A change like that probably deserves a separate patch.
----------------------------------------------------------------
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] asfgit closed pull request #1027: DISPATCH-1960: protocol generic Q2 unblocking API
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027
----------------------------------------------------------------
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] kgiusti commented on a change in pull request #1027: DISPATCH-1960: protocol generic Q2 unblocking API
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027#discussion_r575508066
##########
File path: include/qpid/dispatch/container.h
##########
@@ -228,7 +228,7 @@ void qd_link_close(qd_link_t *link);
void qd_link_detach(qd_link_t *link);
void qd_link_free(qd_link_t *link);
void *qd_link_get_node_context(const qd_link_t *link);
-void qd_link_restart_rx(qd_link_t *link);
+void qd_link_q2_restart_recv(void *context);
Review comment:
Done
----------------------------------------------------------------
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] kgiusti commented on a change in pull request #1027: DISPATCH-1960: protocol generic Q2 unblocking API
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027#discussion_r575508316
##########
File path: include/qpid/dispatch/message.h
##########
@@ -556,12 +555,21 @@ bool qd_message_Q2_holdoff_should_unblock(qd_message_t *msg);
*/
bool qd_message_is_Q2_blocked(const qd_message_t *msg);
Review comment:
Fixed (for the new interfaces only)
----------------------------------------------------------------
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] kgiusti commented on pull request #1027: checkpoint
Posted by GitBox <gi...@apache.org>.
kgiusti commented on pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027#issuecomment-777126228
This is the first part - creating a generic Q2 unblock handler. Follow up commits will use this to enable Q2 backpressure on HTTP/1.x messages.
----------------------------------------------------------------
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 #1027: WIP: generic Q2 unblocking API
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027#discussion_r574599932
##########
File path: include/qpid/dispatch/container.h
##########
@@ -228,7 +228,7 @@ void qd_link_close(qd_link_t *link);
void qd_link_detach(qd_link_t *link);
void qd_link_free(qd_link_t *link);
void *qd_link_get_node_context(const qd_link_t *link);
-void qd_link_restart_rx(qd_link_t *link);
+void qd_link_q2_restart_recv(void *context);
Review comment:
Can we please call this qd_link_q2_restart_receive()
----------------------------------------------------------------
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 #1027: WIP: generic Q2 unblocking API
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027#discussion_r574622069
##########
File path: src/message.c
##########
@@ -2891,3 +2911,21 @@ int qd_message_stream_data_append(qd_message_t *message, qd_buffer_list_t *data)
qd_compose_free(field);
return rc;
}
+
+
+void qd_message_set_Q2_unblocked_handler(qd_message_t *msg,
+ qd_message_Q2_unblocked_handler_t callback,
+ const qd_alloc_safe_ptr_t *ptr)
Review comment:
Can we please please call the *ptr parameter as *sp or *safe_ptr like so - const qd_alloc_safe_ptr_t *safe_ptr OR const qd_alloc_safe_ptr_t *sp
----------------------------------------------------------------
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] kgiusti commented on a change in pull request #1027: DISPATCH-1960: protocol generic Q2 unblocking API
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027#discussion_r575485738
##########
File path: include/qpid/dispatch/message.h
##########
@@ -556,12 +555,21 @@ bool qd_message_Q2_holdoff_should_unblock(qd_message_t *msg);
*/
bool qd_message_is_Q2_blocked(const qd_message_t *msg);
Review comment:
Yeah that's a mess. Uppercase "Q2" in a function name violates our code conventions. I'll update the patch moving all the new names to lower case for consistency. We can change the older names in a follow up patch.
----------------------------------------------------------------
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 #1027: WIP: generic Q2 unblocking API
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027#discussion_r574601957
##########
File path: include/qpid/dispatch/message.h
##########
@@ -556,12 +555,21 @@ bool qd_message_Q2_holdoff_should_unblock(qd_message_t *msg);
*/
bool qd_message_is_Q2_blocked(const qd_message_t *msg);
Review comment:
In some function names Q2 is capitalized, in some function names we have lower case q2 like in qd_message_is_Q2_blocked vs qd_link_q2_restart_recv
----------------------------------------------------------------
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] kgiusti commented on a change in pull request #1027: DISPATCH-1960: protocol generic Q2 unblocking API
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1027:
URL: https://github.com/apache/qpid-dispatch/pull/1027#discussion_r575488021
##########
File path: src/message.c
##########
@@ -2891,3 +2911,21 @@ int qd_message_stream_data_append(qd_message_t *message, qd_buffer_list_t *data)
qd_compose_free(field);
return rc;
}
+
+
+void qd_message_set_Q2_unblocked_handler(qd_message_t *msg,
+ qd_message_Q2_unblocked_handler_t callback,
+ const qd_alloc_safe_ptr_t *ptr)
Review comment:
Yeah, I've refactored this a bit. I'm not crazy about passing in an actual pointer as the context and having the callback also take a pointer. This API would infer that the pointer passed in is handed back, which it isn't.
I want to avoid a pointer here because that raises ownership issues, possibly cross threads. I chose instead a pass by value API so it's clear that ownership of the safe pointer is not transferred but copied.
----------------------------------------------------------------
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