-
Notifications
You must be signed in to change notification settings - Fork 482
DPL: fix routing issues in forwarding #14859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
|
REQUEST FOR PRODUCTION RELEASES: This will add The following labels are available |
|
@shahor02 @davidrohr @Barthelemy @knopers8 this might explain some of the issues you have seen in async reconstruction of pp data on the EPN. There was for sure some flawed logic, I am not sure if it could be triggered in normal circumstances apart from EoS or multi proxy setups when wildcards are involved. Unless you are 100% happy with this, I want to first write a unit test for the new forwarding logic to make sure all my assumptions are correct. |
| // We need to find the forward route only for the first | ||
| // part of a split payload. All the others will use the same. | ||
| // but always check if we have a sequence of multiple payloads | ||
| cachedForwardingChoices.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand, how does the caching work for multi-parts now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed the variable to avoid confusion. I think the ability to cache the forwarding choice was either wrong or only correct in a past where wildcards were not there. To exemplify:
- Assume you have a wildcard InputSpec for
SRC/*. - This will match
SRC/AandSRC/Band put them in the same messageSet. - There is no guarantee that both
SRC/AandSRC/Bare to be sent to the same place.
The above only worked so far because I assume in general a wildcard is used for sinks and therefore it is last.
For multiparts, it is enough to iterate over the messageSet.getNumberOfPayloads(pi) as it's done later in the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see what you mean now. You are talking about the split payloads in the form:
[Header1][payload0][header1][payload1]... etc
indeed for those I need to have the cache to stay around in the new version... Will do it in a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated with the old logic and improved description.
1677bb0 to
57e7546
Compare
|
Error while checking build/O2/fullCI_slc9 for 57e7546 at 2025-11-29 22:54: Full log here. |
|
Error while checking build/O2/fullCI_slc9 for aa79411 at 2025-12-08 15:06: Full log here. |
Use a single helper function to improve readability.
If one (header, payload, ...) tuple in a MessageSet was to be copied, all the subsequent ones would have been copied. If one (header, payload, ...) tuple got redirected to more than one destination, all the subsequent ones would have been redirected there.
If one (header, payload, ...) tuple in a MessageSet was to be copied,
all the subsequent ones would have been copied.
If one (header, payload, ...) tuple got redirected to more than one destination,
all the subsequent ones would have been redirected there.
Stack created with Sapling. Best reviewed with ReviewStack.