[
https://issues.apache.org/jira/browse/DISPATCH-1780?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17226259#comment-17226259
]
ASF GitHub Bot commented on DISPATCH-1780:
------------------------------------------
kgiusti commented on a change in pull request #909:
URL: https://github.com/apache/qpid-dispatch/pull/909#discussion_r516675946
##########
File path: python/qpid_dispatch/management/qdrouter.json
##########
@@ -1165,6 +1177,18 @@
"default": "HTTP1",
"required": false,
"create": true
+ },
+ "aggregation": {
+ "type": "string",
Review comment:
Ditto above comment
##########
File path: python/qpid_dispatch/management/qdrouter.json
##########
@@ -1124,6 +1124,18 @@
"default": "HTTP1",
"required": false,
"create": true
+ },
+ "aggregation": {
+ "type": "string",
Review comment:
Would it be possible to use a 'type' list of allowed values here like we
do for protocolVersion?
##########
File path: src/adaptors/http1/http1_codec.c
##########
@@ -1528,6 +1533,62 @@ static inline void
_flush_headers(h1_codec_request_state_t *hrs, struct encoder_
}
}
+int h1_codec_tx_begin_multipart(h1_codec_request_state_t *hrs)
+{
+ h1_codec_connection_t *conn = h1_codec_request_state_get_connection(hrs);
+ struct encoder_t *encoder = &conn->encoder;
+ encoder->boundary_marker = (char*) malloc(QD_DISCRIMINATOR_SIZE + 2);
Review comment:
Should the boundary_marker be freed in the encoder reset function? or
perhaps during end_multipart?
##########
File path: src/adaptors/http1/http1_server.c
##########
@@ -1056,11 +1063,10 @@ void
qdr_http1_server_core_link_flow(qdr_http1_adaptor_t *adaptor,
rmsg->dlv = qdr_link_deliver_to(hconn->in_link, rmsg->msg, 0,
addr, false, 0, 0, 0, 0);
qdr_delivery_set_context(rmsg->dlv, (void*) hreq);
rmsg->msg = 0;
- if (!rmsg->rx_complete) {
+ if (!rmsg->rx_complete || hconn->cfg.aggregation !=
QD_AGGREGATION_NONE) {
// stop here since response must be complete before we can
deliver the next one.
Review comment:
Why exit sooner than rx_complete if aggregation? comment needs updating
##########
File path: src/adaptors/http1/http1_client.c
##########
@@ -1218,6 +1406,26 @@ uint64_t
qdr_http1_client_core_link_deliver(qdr_http1_adaptor_t *adaptor,
_client_response_msg_t *rmsg = DEQ_TAIL(hreq->responses);
assert(rmsg && rmsg->dlv == delivery);
+ if (hconn->cfg.aggregation != QD_AGGREGATION_NONE) {
Review comment:
Pls add comment explaining that responses are saved and not encoded
until after the request has been accepted.
##########
File path: src/adaptors/http1/http1_client.c
##########
@@ -541,7 +546,12 @@ static void _client_tx_buffers_cb(h1_codec_request_state_t
*hrs, qd_buffer_list_
// responses are decoded one at a time - the current response it at the
// tail of the response list
- _client_response_msg_t *rmsg = DEQ_TAIL(hreq->responses);
+ _client_response_msg_t *rmsg;
+ if (hconn->cfg.aggregation == QD_AGGREGATION_NONE) {
+ rmsg = DEQ_TAIL(hreq->responses);
+ } else {
+ rmsg = DEQ_HEAD(hreq->responses);
Review comment:
My brain exploded. Why the _head_ response message?
##########
File path: src/adaptors/http1/http1_client.c
##########
@@ -575,7 +585,12 @@ static void
_client_tx_stream_data_cb(h1_codec_request_state_t *hrs, qd_message_
// responses are decoded one at a time - the current response it at the
// tail of the response list
- _client_response_msg_t *rmsg = DEQ_TAIL(hreq->responses);
+ _client_response_msg_t *rmsg;
+ if (hconn->cfg.aggregation == QD_AGGREGATION_NONE) {
+ rmsg = DEQ_TAIL(hreq->responses);
+ } else {
+ rmsg = DEQ_HEAD(hreq->responses);
Review comment:
Ouch - brain explodes again!
##########
File path: src/adaptors/http1/http1_client.c
##########
@@ -942,6 +1126,10 @@ void
qdr_http1_client_core_delivery_update(qdr_http1_adaptor_t *adaptor,
qdr_http1_error_response(&hreq->base, 503, "Service
Unavailable");
Review comment:
Note that qdr_http1_error_response() calls h1_codec_tx_done which closes
the encoder - will that conflict with the new aggregation code below? Should
the new code be skipped if !PN_ACCEPTED?
##########
File path: src/adaptors/http1/http1_client.c
##########
@@ -541,7 +546,12 @@ static void _client_tx_buffers_cb(h1_codec_request_state_t
*hrs, qd_buffer_list_
// responses are decoded one at a time - the current response it at the
// tail of the response list
- _client_response_msg_t *rmsg = DEQ_TAIL(hreq->responses);
+ _client_response_msg_t *rmsg;
+ if (hconn->cfg.aggregation == QD_AGGREGATION_NONE) {
+ rmsg = DEQ_TAIL(hreq->responses);
+ } else {
+ rmsg = DEQ_HEAD(hreq->responses);
Review comment:
Update: now I understand - we buffer all responses and hold off writing
them until after request is acked. Need to update comment line 546.
##########
File path: src/adaptors/http1/http1_client.c
##########
@@ -575,7 +585,12 @@ static void
_client_tx_stream_data_cb(h1_codec_request_state_t *hrs, qd_message_
// responses are decoded one at a time - the current response it at the
// tail of the response list
- _client_response_msg_t *rmsg = DEQ_TAIL(hreq->responses);
+ _client_response_msg_t *rmsg;
+ if (hconn->cfg.aggregation == QD_AGGREGATION_NONE) {
+ rmsg = DEQ_TAIL(hreq->responses);
+ } else {
+ rmsg = DEQ_HEAD(hreq->responses);
Review comment:
Brain re-assembled - update comment on line 585.
----------------------------------------------------------------
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:
us...@infra.apache.org
> multicast support for http 1.1 adaptor
> --------------------------------------
>
> Key: DISPATCH-1780
> URL: https://issues.apache.org/jira/browse/DISPATCH-1780
> Project: Qpid Dispatch
> Issue Type: Improvement
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Priority: Major
> Fix For: 1.15.0
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
