-
Notifications
You must be signed in to change notification settings - Fork 516
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
Fix for mediator load testing race condition when scaling horizontally #2009
Conversation
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #2009 +/- ##
=======================================
Coverage 93.51% 93.51%
=======================================
Files 539 539
Lines 34608 34625 +17
=======================================
+ Hits 32363 32380 +17
Misses 2245 2245 |
@ianco We can also use the Redis cache across scaled-up mediators to track if |
@shaangill025 I think using the redis (or kafka) queue is a better approach than the hack-y solution I put in. However unless we make queues a requirement (as @swcurran suggested) we also have to have a fall-back solution for when there are no queues deployed. (I think this problem can occur even if there is just a single instance of the mediator, although much less likely.) For a redis/kafka solution, I think all that aca-py need to do - if there is no routing record found - is just leave the message on the queue with a "retry later" flag, I don't think there's a reason to do any additional checking. (Plus I'm not sure of the timing - is it possible that Mediator_B will try to find the routing record before Mediator_A even gets the request to add the routing record? I'm not sure of the root cause of this problem so we would need to assume this is a possibility.) |
Kudos, SonarCloud Quality Gate passed! |
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.
Confirmed and we agree this should be merged. A subsequent update for adding queuing support in this area may be needed, but that can follow.
Pause and retry if a routing record is not found