From 42eddc6429a7768a573b5d51d5fed6f381a55cff Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Tue, 26 Apr 2022 16:49:55 -0400 Subject: [PATCH] LP#1970469: fix responses streamed out of order due to chunking If a streaming method starts off sending a couple small responses followed by a large one, the responses can end up received out of order under this scenario: - The initial small responses get stashed to be sent out as a bundle - The large response, if big enough to require chunking, gets sent out as a set of partial responses. However, any messages queued up to go out as a bundle remain in the queue. - When the method completes, or if further responses result in exceeding the maximum bundle size, the queued messages get sent _after_ the chunked response. This affects services written in C and Perl. To test ------- C service ========= [1] Add the following stored procedure to an Evergreen database: CREATE OR REPLACE FUNCTION public.lp1970469_sample() RETURNS SETOF TEXT AS $$ BEGIN RETURN NEXT 'abc'; RETURN NEXT 'def'; RETURN NEXT rpad('long', 65000, 'x'); RETURN NEXT 'xyz'; RETURN; END; $$ LANGUAGE PLPGSQL IMMUTABLE; [2] Run the following srfsh command: srfsh# open-ils.cstore open-ils.cstore.json_query.atomic {"from":["public.lp1970469_sample"]} [3] Note that longxxxxx* response is returned first, followed by abc, def, and xyz. [4] Apply the patch and repeat step 2. This time, the responses should be returned in the expected order. Perl service ============ [1] In an Evergreen database, attach a couple hundred items to a record. [2] Run the following srfsh command: srfsh# request open-ils.supercat open-ils.supercat.record.holdings_xml.retrieve BIBID, "-", null, 1 [3] If enough items were attached, the first response streamed will start with a element rather than the element and the overall response will not be well-formed XML. [4] Apply the patch and repeat step 2. This time, the parts of the response should be in the expected order. Note that this test scenario models an Evergreen glitch seen in the wild that causes requests for https://EGSERVER//opac/extras/supercat/retrieve/marcxml-full/record/BIBID to fall if there are a lot of items attached to the bib. Signed-off-by: Galen Charlton Signed-off-by: Mike Rylander --- src/libopensrf/osrf_application.c | 5 +++++ src/perl/lib/OpenSRF/AppSession.pm | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/libopensrf/osrf_application.c b/src/libopensrf/osrf_application.c index 4a5f53e..b1a149f 100644 --- a/src/libopensrf/osrf_application.c +++ b/src/libopensrf/osrf_application.c @@ -746,6 +746,11 @@ static int _osrfAppRespond( osrfMethodContext* ctx, const jsonObject* data, int // chunking -- response message exceeds max message size. // break it up into chunks for partial delivery + // but first, send out any any messages that may have + // been queued for bundling + if( flush_responses( ctx->session, ctx->session->outbuf )) + return -1; + osrfSendChunkedResult(ctx->session, ctx->request, data_str, raw_size, chunk_size); diff --git a/src/perl/lib/OpenSRF/AppSession.pm b/src/perl/lib/OpenSRF/AppSession.pm index 1356178..603ba3c 100644 --- a/src/perl/lib/OpenSRF/AppSession.pm +++ b/src/perl/lib/OpenSRF/AppSession.pm @@ -1082,6 +1082,18 @@ sub respond { } if ($raw_length > $chunk_size) { # send partials ("chunking") + # First, send out any messages queued up to be sent as + # a bundle; otherwise, the chunked message will get sent + # out of order + if ($self->{current_bundle_count} > 0) { + # ignore any errors; if there's a problem, it presumably + # will be reported when the partial-complete message + # is sent + $self->session->send( @{$self->{current_bundle}}, $self->threadTrace); + $self->{current_bundle} = []; + $self->{current_bundle_size} = 0; + $self->{current_bundle_count} = 0; + } my $num_bytes = length(Encode::encode_utf8($str)); for (my $i = 0; $i < $num_bytes; $i += $chunk_size) { $response = new OpenSRF::DomainObject::oilsResult::Partial; -- 2.43.2