GH-7925: Make message history header as mutable #8980
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes: spring-projects#7925 The `MessageHistory.write()` creates not only a new instance of the `MessageHistory`, but also a new copy of the whole message. This significantly impacts the performance when we have too many components to track * Make `MessageHistory` as append-only container and create a new instance (plus message) only on the first track when no prior history is present * Change `WireTap`, `BroadcastingDispatcher`, `AbstractMessageRouter` and `AbstractMessageSplitter` to use a new `AbstractIntegrationMessageBuilder.cloneMessageHistoryIfAny()` API for every branch a message is produced. Essentially, create a new message with copy of the message history to let that downstream sub-flow have its own trace * Modify failed unit tests for a new logic where message history header is not immutable anymore * This also fixes an `AbstractMessageSplitter` for propagating its track into messages it emits
sobychacko
requested changes
Mar 5, 2024
| NOTE: Prior to version 6.3, the message history header was immutable (you cannot re-write history): every single track created not only new instance of the `MessageHistory`, but a fully new message copy. | ||
| Now it works in append-only mode: the first track creates a new message with a new `MessageHistory` container. | ||
| All the rest `MessageHistory.write()` calls add new entries to existing header - and no new message created. | ||
| This significantly improves an application performance. |
There was a problem hiding this comment.
I'm not sure what is this application in this context.
My thought was about any application which uses message history.
Maybe you meant the instead?
There was a problem hiding this comment.
oh ya, i meant the. sorry.
| Now it works in append-only mode: the first track creates a new message with a new `MessageHistory` container. | ||
| All the rest `MessageHistory.write()` calls add new entries to existing header - and no new message created. | ||
| This significantly improves an application performance. | ||
| All the components in the framework, where same message can be sent to several consumers (`PublishSubscribeChannel`, `AbstractMessageRouter`, `WireTap` etc.) (or splitter produces several outputs based on input message), are now cloning an existing `MessageHistory` header into those new messages. |
There was a problem hiding this comment.
I don't see the need for the parenthesis around the sentence, or splitter produces.... Also or where splitter produces...?
There was a problem hiding this comment.
Is it OK to have several where in the sequence?
I mean we already have one where in the beginning.
This or is just one more in the sequence.
So, might just no parenthesis and that's it?
|
@artembilan Looks great based on our code walkthrough. Some very minor nit-picks on the docs. That's all. |
…blish * Fix typos in docs
sobychacko
approved these changes
Mar 5, 2024
|
The build is 🟢 . Thanks |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #7925
The
MessageHistory.write()creates not only a new instance of theMessageHistory, but also a new copy of the whole message.This significantly impacts the performance when we have too many components to track
MessageHistoryas append-only container and create a new instance (plus message) only on the first track when no prior history is presentWireTap,BroadcastingDispatcher,AbstractMessageRouterandAbstractMessageSplitterto use a newAbstractIntegrationMessageBuilder.cloneMessageHistoryIfAny()API for every branch a message is produced.Essentially, create a new message with copy of the message history to let that downstream sub-flow have its own trace
AbstractMessageSplitterfor propagating its track into messages it emits