Bug 449003 - request-response example: support clean shutdown of server apps
request-response example: support clean shutdown of server apps
Status: CLOSED WONTFIX
Product: Red Hat Enterprise MRG
Classification: Red Hat
Component: Messaging_Programming_Reference (Show other bugs)
beta
All Linux
medium Severity low
: 2.0
: ---
Assigned To: Alison Young
ecs-bugs
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-29 16:35 EDT by Alan Conway
Modified: 2012-09-21 09:02 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-12-06 06:28:47 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Alan Conway 2008-05-29 16:35:29 EDT
Gordon Sim wrote:
> qpid-test-cruisecontrol@redhat.com wrote:
>> View results here -> 
>>
http://qpid-test.lab.bos.redhat.com:8080/buildresults/qpid-cpp-trunk?log=log20080529155220

>>
>>
> 
> FYI: This is the same error seen once before. We definitely have an 
> intermittent problem, will need to dig in deeper to try and figure out 
> what it is.
> 
> 
> == examples/request-response/verify
> *** verify.out 2008-05-29 16:03:45.000000000 -0400
> --- verify.in 2008-02-09 00:00:07.000000000 -0500
> ***************
> *** 13,19 ****
> ==== server.out | remove_uuid
> Activating request queue listener for: request
> Waiting for requests
> - Request: And the mome raths outgrabe. (client)
> Request: Twas brillig, and the slithy toves (client)
> Request: Did gire and gymble in the wabe. (client)
> Request: All mimsy were the borogroves, (client)
> --- 13,18 ----
> FAIL

More than once. These symptoms used to be easily reproducible by running one of
the cpp/python combinations in a loop (forget which one)

The last time around we discoved that the broker was not getting the last
message completed from the client. The fix (which made the race much less
reproducible at least!) was to add a sync() to Dispatcher::run() just before it
returns.

Thinking about it though, that's clearly not the correct fix (but the added
delay makes the race less likely.) sync() is ensuring that the *broker* has
completed all outsanding commands. We need to do the opposite: ensure the
*client* has sent completed's for all outstanding commands *and knows them to be
received*.

My brain is too fried to fix this right now, but there's a combination of
session controls that will achieve this. There's some discussion on the AMQP
spec about clearing session state before shutdown. We should do this always on
shutdown, and possibly add some of this to sync() or provide a second
synchronization function to do it.
Comment 1 Gordon Sim 2008-05-30 09:22:56 EDT
I believe the actual cause is that the python 'server' application is killed by
the verify script before it has time to send the acknowledgement. The temporary
solution in the verification is to add a short sleep before the kill (which I
have done).

The better solution however is to alter the examples to support a clean shutdown
of the server at the clients request. From aconway:

"The server fix:
  if (request.body == "shutdown") break;

Client fix: after sending requests:
  session.message_transfer(destination="amq.direct",
message=Message(message_properties, delivery_properties, "shutdown"))

Test  harness fix: take out the kill, wait for the server to exit like other
examples.

And equivalent changes for C++ & Java examples. "
Comment 2 Jonathan Robie 2008-11-13 10:45:37 EST
We've added and removed code to shut down this way at least once, and I think perhaps more than once. I don't see that code in the example now.

Have we changed our minds?

Jonathan



void Listener::received(Message& request) {
    Message response;

    // Get routing key for response from the request's replyTo property
    string routingKey;

    if (request.getMessageProperties().hasReplyTo()) {
        routingKey = request.getMessageProperties().getReplyTo().getRoutingKey();
    } else {
        std::cout << "Error: " << "No routing key for request (" << request.getData() << ")" << std::endl;
        return;
    }

    std::cout << "Request: " << request.getData() << "  (" <<routingKey << ")" << std::endl;

    // Transform message content to upper case
    std::string s = request.getData();
    std::transform (s.begin(), s.end(), s.begin(), toupper);
    response.setData(s);

    // Send it back to the user
    response.getDeliveryProperties().setRoutingKey(routingKey);

    // Asynchronous transfer sends messages as quickly as
    // possible without waiting for confirmation.
    asyncSession.messageTransfer(arg::content=response, arg::destination="amq.direct");
}
Comment 4 Alison Young 2010-12-03 01:48:16 EST
Hi,

Can the code please be reviewed for this bug and advice returned regarding whether this example is still relevant for the Programming in Apache Qpid book?

Thanks, Alison
Comment 5 Alison Young 2010-12-05 18:57:55 EST
Hi,  

Can I please get confirmation that this bug is still relevant and required for inclusion in MRG 2.0 and also that the code used is still correct.

Thanks,
Alison
Comment 6 Gordon Sim 2010-12-06 06:28:47 EST
Not a doc bug really anyway; more an enhancement to the automated testing of the old examples. This is no longer needed as these are being removed from trunk.

Note You need to log in before you can comment on or make changes to this bug.