[
https://issues.apache.org/jira/browse/QPID-4178?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gordon Sim updated QPID-4178:
-----------------------------
Attachment: refactor_store_impact.patch
Slight tweak to previous patch, avoiding unintentional commenting out of
subdirs for tests.
> qpidd refactor
> --------------
>
> Key: QPID-4178
> URL: https://issues.apache.org/jira/browse/QPID-4178
> Project: Qpid
> Issue Type: Improvement
> Components: C++ Broker
> Reporter: Gordon Sim
> Assignee: Gordon Sim
> Fix For: 0.19
>
> Attachments: refactor_store_impact.patch, refactor_store_impact.patch
>
>
> == Background ==
> I've been looking at what would be required to get AMQP 1.0 support in
> the qpidd broker (using proton-c). In that context I felt there was a
> need to refactor the broker code, particularly that part that would be
> shared between different protocol versions. Part of the motivation was
> clearer separation of 0-10 specific logic, so that 1.0 logic could be
> introduced as an alternative. However part of it was also simply the
> recognition of some long-standing problems that we have never stopped
> to address.
> So, here is a patch representing my ideas on what is needed. This is
> a little stale again (patch was generated against r13613342) and
> needs (yet) another rebase. However it is getting to the point where I'll be
> asking to commit it soon, so if anyone has feedback, now is the time to give
> it!
> == Key Changes ==
> qpid::broker::Message
> This is now supposed to be a protocol neutral representation of a
> message. It no longer exposes qpid::framing::FrameSet. It can be based
> on data received in different encodings (this patch only includes the
> existing 0-10 encoding).
> The immutable, sharable state is separated from the mutable
> queue-specific state. Messages themselves are no longer held through a
> shared pointer but are passed by reference or copied if needed. The
> immutable state (essentially the data as received) *is* still shared
> and referenced internally through an intrusive pointer. There is no
> longer a message level lock. A message instance is 'owned' by
> someother entity (usually the queue it is on) which controls
> concurrent access/modification if necessary.
> The persistence context is a separate part of the message
> also. Currently that can be shared between two message instances if
> desired.
> qpid::broker::Messages
> Switched from using qpid::broker::QueuedMessage (which relied on
> shared pointer to message itself and made sequence number the explicit
> - and only - way to refer to a specific message) to using modified
> Message class directly and a new qpid::broker::QueueCursor.
> The cursor is opaque outside the Messages implementation to which it
> relates. It provides a way to refer to a specific message (without
> directly using sequence number, though at present that is what is used
> 'under the covers') and/or to track progress through a sequence of
> messages (for consumers or other iterating entities).
> I.e. its an iterator to a Message within its containing Messages
> instance that is not invalidated by changes to that container.
> A Messages instance *owns* the Message instances within it. Other
> classes access this through a reference or (raw) pointer, or if needed
> copy it (the immutable part can be - and is - safely shared).
> The codepath for browse/consume is a lot more unified now. You use a
> cursor and call Messages::next() in each case. This also lays the
> foundation for selectors.
> The simplified Messages interface led to a simplied
> MessageDistributor. There is still a little more to do to clarify
> these separate roles (or indeed perhaps unify them?) but more on that
> later.
> qpid::broker::amqp_0_10::MessageTransfer
> This represents the familiar 0-10 encoding of a message. This class is
> broadly similar to the old Message class, based on a FrameSet. However
> it represents the shared and essentially immutable state. The
> sendHeader() method now explicitly takes a copy of the original
> headers and adds to it or otherwise modifies it if needed (e.g. for
> redelivered flag, ttl, annotations etc).
> [Ideally I'd like to move more of the 0-10 specific classes out of
> qpid::broker and into qpid::broker::amqp_0_10, but that has no
> functional relevance so I've left existing classes alone for now.]
> qpid::broker::Consumer
> The deliver() method now takes a QueueCursor (representing a 'handle'
> to this message for use in subsequent operations such as accept,
> relese etc) and a *constant reference* to the Message itself
> (i.e. consumers can't alter the state of the message on the queue
> directly, but only through operations on the queue itself).
> qpid::broker::QueueRegistry
> The actual queue creation has been pulled out into a base class,
> QueueFactory. The actual class of the Queue returned can now be varied
> and there are two subclasses in the current patch. The first is a
> replacement for the ring policy logic, whereby messages are removed
> from the queue in order to keep the queue from growing above a
> configured limit. The second is for last value queues and simply pulls
> the special case behaviour out of the common code path.
> The handling of queue configuration has also been made cleaner and
> more uniform, based on the QueueSettings class.
> qpid::broker::QueuePolicy
> This class has been removed. There is a new QueueDepth utility used
> for configuring limits, tracking current depth and testing the latter
> against the former. This is used directly by Queue. The behaviour at
> the limit can be varied by subclassing queue.
> == Limitations etc ==
> clustering
> This breaks clustering. Indeed it will not compile unless clustering
> is disabled (--without-cpg in configure). Keeping the cluster code in
> sync was distracting me from the core goal, given its entanglement
> with the broker code.
> My assumption is that the new ha code will eventually replace the
> cluster anyway and the amount of change that would be required to get
> the cluster working with this refactor may not be worth it and may in
> fact undermine its stability anyway (which seem the only good argument
> for using it).
> I don't believe there is anything insurmountable to do to re-enable
> cluster if that was desired however, just a fair chunk more work that I would
> rather not do if I can avoid it.
> old & nasty features removed
> I have removed support for flow to disk, the legacy version of lvq
> with two modes (the updated version of lvq is of course still
> functional), the last-man-standing persistence in clustering and the
> old async queue replication. They are really quite horribly
> implemented and/or are no longer necessary in my view.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
