qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pavel Moravec" <pmora...@redhat.com>
Subject Review Request 23545: [C++ client] Allow consumer flow control based on #bytes
Date Wed, 16 Jul 2014 09:59:00 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23545/
-----------------------------------------------------------

Review request for qpid, Gordon Sim and mick goulish.


Bugs: QPID-5895
    https://issues.apache.org/jira/browse/QPID-5895


Repository: qpid


Description
-------

AMQP 0-10:
byteCredit in 0-10 ReceiverImpl renamed to byteWindow.

Both (message and byte) window/capacity operate independently on each other.

I had to add SessionImpl::getMaxPrefetchedBytes to get around pre-declared class Connection.


AMQP 1.0:
If max-prefetched-bytes to be used, I fixed maxFrameForBytePrefetch in 'open' performative
to 8k. That should be enough high for most of messages and should provide sufficient granularity
of incoming-window.

Note, pn_session_set_incoming_capacity sets session's byte capacity whereas incoming_window
is returned by proton method:

size_t pn_session_incoming_window(pn_session_t *ssn)
{
  uint32_t size = ssn->connection->transport->local_max_frame;
  if (!size) {
    return 2147483647; // biggest legal value
  } else {
    return (ssn->incoming_capacity - ssn->incoming_bytes)/size;
  }
}

If a user sets max-prefetched-bytes below 8k, the client will automatically increase it to
8k with some warning (otherwise incoming-window would be zero and no consumer would get a
single message).

Note, the 1.0 patch does not work due to QPID-5896 I plan to fix first.


Diffs
-----

  /trunk/qpid/cpp/src/qpid/client/amqp0_10/ConnectionImpl.h 1609704 
  /trunk/qpid/cpp/src/qpid/client/amqp0_10/ConnectionImpl.cpp 1609704 
  /trunk/qpid/cpp/src/qpid/client/amqp0_10/ReceiverImpl.h 1609704 
  /trunk/qpid/cpp/src/qpid/client/amqp0_10/ReceiverImpl.cpp 1609704 
  /trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionImpl.h 1609704 
  /trunk/qpid/cpp/src/qpid/client/amqp0_10/SessionImpl.cpp 1609704 
  /trunk/qpid/cpp/src/qpid/messaging/ConnectionOptions.h 1609704 
  /trunk/qpid/cpp/src/qpid/messaging/ConnectionOptions.cpp 1609704 
  /trunk/qpid/cpp/src/qpid/messaging/amqp/ConnectionContext.cpp 1609704 

Diff: https://reviews.apache.org/r/23545/diff/


Testing
-------

Automated tests passed (well, TestParseTcpIPv6 failed but that fails on trunk as well - already
reported).

Manual tests on 0-10 passed as well. On 1.0, they would work (per tcpdump, both max-frame-size
in 'open' performative and incoming-window in 'begin'), but only after QPID-5896 is fixed
:-/ .

(I will post the patch only after QPID-5896 is fixed (and after ACKs from this review, of
course :) )


Thanks,

Pavel Moravec


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message