[
https://issues.apache.org/jira/browse/QPID-3859?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13212834#comment-13212834
]
jirapos...@reviews.apache.org commented on QPID-3859:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2828/#review5246
-----------------------------------------------------------
There's a whole lot of code here and I don't understand it well enough to
actually review it - below are some random comments that occurred to me whilst
reading through.
I would say that the amount of code seems disproportionate to the actual
functionality, but that is clearly subjective. However the code really isn't
well enough explained to be able to understand what the pieces do and how they
relate.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/Prong.h
<https://reviews.apache.org/r/2828/#comment11457>
The name Prong seems inscrutable, especially without any explanation what
it means.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/Prong.cpp
<https://reviews.apache.org/r/2828/#comment11456>
Why invent a new code convention?
elsewhere in the C++ code we use
Class::Class() :
member1(0),
member2(0),
....
I know your code convention is the correct one but we have 5 years of code
for you to change if you think it's better than the previous!
trunk/qpid/cpp/bindings/qpid/ruby/examples/drain.rb
<https://reviews.apache.org/r/2828/#comment11458>
This seems like a serious problem, perhaps it needs to be addressed before
putting this change to trunk?
trunk/qpid/cpp/bindings/qpid/ruby/examples/spout.rb
<https://reviews.apache.org/r/2828/#comment11459>
As previous, having to sleep for 100ms seems to indicate a severe problem
being swept under the rug. Especially in an API meant to be nonblocking.
trunk/qpid/cpp/include/qpid/messaging/Tracker.h
<https://reviews.apache.org/r/2828/#comment11455>
I can't tell from this class what a "Tracker" is supposed to do, or how to
use it - This is externally visible API so needs appropriate documentation.
trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionTracker.cpp
<https://reviews.apache.org/r/2828/#comment11463>
Why the dead code in a comment? If it needs to be there say why. If not
delete it, but tell me why you getDeadline and then ignore the answer.
trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionTracker.cpp
<https://reviews.apache.org/r/2828/#comment11464>
As above seems like this is dead code entirely.
trunk/qpid/cpp/src/qpid/sys/BlockingQueue.h
<https://reviews.apache.org/r/2828/#comment11465>
I don't understand the design rationale here surely the whole point of the
queue being waitable is to have another component wait for things to happen to
the queue.
If you want to add some sort of callback functionality surely you should
add it by having something wait for the queue and then do the appropriate
callback.
That seems more appropriate separation of responsibilities to me.
- Andrew
On 2012-02-20 15:53:08, Darryl Pierce wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2828/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-02-20 15:53:08)
bq.
bq.
bq. Review request for Andrew Stitcher, Alan Conway, Gordon Sim, Kenneth
Giusti, and Rafael Schloming.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This first pass has full integration of the Tracker type with the Ruby
bindings to provide a non-blocking means for responding to incoming messages.
bq.
bq. After a Receiver is created, a call to Qpid::Messaging.receive will wait
for the next message to become available on it. When one is received, a
provided lambda function is invoked and the receiver passed to it. The message
can then be retrieved, acknowledged, etc.
bq.
bq.
bq. This addresses bug QPID-3859.
bq. https://issues.apache.org/jira/browse/QPID-3859
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/qpid/cpp/bindings/qpid/CMakeLists.txt 1243858
bq. trunk/qpid/cpp/bindings/qpid/nonblockio/CMakeLists.txt PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/Prong.h
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/Prong.cpp
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/TrackerAdaptor.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/TrackerAdaptor.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/TrackerAdaptorImpl.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/TrackerAdaptorImpl.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/TrackerEventHandler.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/TrackerEventHandler.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/Acknowledge.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/Acknowledge.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/AcknowledgeImpl.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/AcknowledgeImpl.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/BaseThreadedEventHandler.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/BaseThreadedEventHandler.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/BaseTrackerEventHandler.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/BaseTrackerEventHandler.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/NextReceiver.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/NextReceiver.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/NextReceiverImpl.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/NextReceiverImpl.cpp
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/Receive.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/Receive.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/ReceiveImpl.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/ReceiveImpl.cpp
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/Send.h
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/Send.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/SendImpl.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/SendImpl.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/SessionSync.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/SessionSync.cpp
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/SessionSyncImpl.h
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/nonblockio/qpid/messaging/synchio/SessionSyncImpl.cpp
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/qpid.i 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/CMakeLists.txt 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/README.rdoc 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/Rakefile 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/examples/drain.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/examples/map_receiver.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/examples/spout.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/ext/nonblockio/extconf.rb PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/ruby/ext/nonblockio/nonblockio.h
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/ruby/ext/nonblockio/nonblockio.c
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/ruby/ext/nonblockio/qpid_receiver.c
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/ruby/ext/nonblockio/qpid_sender.c
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/ruby/ext/nonblockio/qpid_session.c
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/ruby/ext/nonblockio/qpid_utils.c
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/ruby/ext/nonblockio/test_next_receiver.rb
PRE-CREATION
bq.
trunk/qpid/cpp/bindings/qpid/ruby/ext/nonblockio/test_receiver_get_and_fetch.rb
PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/ruby/features/receiving_a_message.feature
1243858
bq.
trunk/qpid/cpp/bindings/qpid/ruby/features/step_definitions/receiver_steps.rb
1243858
bq.
trunk/qpid/cpp/bindings/qpid/ruby/features/step_definitions/session_steps.rb
1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/lib/qpid.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/lib/qpid/address.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/lib/qpid/connection.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/lib/qpid/encoding.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/lib/qpid/message.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/lib/qpid/receiver.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/lib/qpid/sender.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/lib/qpid/session.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/lib/qpid/utils.rb PRE-CREATION
bq. trunk/qpid/cpp/bindings/qpid/ruby/lib/qpid/version.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/ruby.i 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/spec/qpid/encoding_spec.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/spec/qpid/message_spec.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/spec/qpid/receiver_spec.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/spec/qpid/sender_spec.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/spec/qpid/session_spec.rb 1243858
bq. trunk/qpid/cpp/bindings/qpid/ruby/spec/spec_helper.rb 1243858
bq. trunk/qpid/cpp/examples/messaging/CMakeLists.txt 1243858
bq. trunk/qpid/cpp/examples/messaging/Makefile.am 1243858
bq. trunk/qpid/cpp/examples/messaging/extra_dist/Makefile 1243858
bq. trunk/qpid/cpp/examples/messaging/non_blocking.cpp PRE-CREATION
bq. trunk/qpid/cpp/include/qpid/messaging/Session.h 1243858
bq. trunk/qpid/cpp/include/qpid/messaging/Tracker.h PRE-CREATION
bq. trunk/qpid/cpp/src/CMakeLists.txt 1243858
bq. trunk/qpid/cpp/src/Makefile.am 1243858
bq. trunk/qpid/cpp/src/qpid/client/SessionImpl.h 1243858
bq. trunk/qpid/cpp/src/qpid/client/SessionImpl.cpp 1243858
bq. trunk/qpid/cpp/src/qpid/client/amqp0_10/SenderImpl.cpp 1243858
bq. trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionImpl.h 1243858
bq. trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionImpl.cpp 1243858
bq. trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionTracker.h PRE-CREATION
bq. trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionTracker.cpp PRE-CREATION
bq. trunk/qpid/cpp/src/qpid/messaging/Session.cpp 1243858
bq. trunk/qpid/cpp/src/qpid/messaging/SessionImpl.h 1243858
bq. trunk/qpid/cpp/src/qpid/messaging/Tracker.cpp PRE-CREATION
bq. trunk/qpid/cpp/src/qpid/sys/BlockingQueue.h 1243858
bq.
bq. Diff: https://reviews.apache.org/r/2828/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Darryl
bq.
bq.
> Provide non-blocking I/O functionality to the Ruby APIs
> -------------------------------------------------------
>
> Key: QPID-3859
> URL: https://issues.apache.org/jira/browse/QPID-3859
> Project: Qpid
> Issue Type: Improvement
> Components: Ruby Client
> Reporter: Darryl L. Pierce
> Priority: Critical
>
> Provide functionality that overcomes the limitation of the Ruby global
> interpreter. Prevent the Ruby VM from become become unresponsive when a
> blocking I/O call is made so that other Ruby threads can continue to execute
> while the I/O continues.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscr...@qpid.apache.org
