Bug 449003

Summary:	request-response example: support clean shutdown of server apps
Product:	Red Hat Enterprise MRG	Reporter:	Alan Conway <aconway>
Component:	Messaging_Programming_Reference	Assignee:	Alison Young <alyoung>
Status:	CLOSED WONTFIX	QA Contact:	ecs-bugs
Severity:	low	Docs Contact:
Priority:	medium
Version:	beta	CC:	gsim, iboverma, lbrindle
Target Milestone:	2.0
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2010-12-06 11:28:47 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Alan Conway 2008-05-29 20:35:29 UTC

Gordon Sim wrote:
> qpid-test-cruisecontrol 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 13:22:56 UTC

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 15:45:37 UTC

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 06:48:16 UTC

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 23:57:55 UTC

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 11:28:47 UTC

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.