Red Hat Bugzilla – Bug 449003
request-response example: support clean shutdown of server apps
Last modified: 2012-09-21 09:02:00 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.
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. "
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"); }
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
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
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.