Skip to content

#1667 fix#1668

Merged
jeremydmiller merged 1 commit intoJasperFx:mainfrom
migajek:main
Sep 3, 2025
Merged

#1667 fix#1668
jeremydmiller merged 1 commit intoJasperFx:mainfrom
migajek:main

Conversation

@migajek
Copy link

@migajek migajek commented Sep 2, 2025

Please note - I lack ImTools experience, so I'm not sure ImTools + lock is still better choice over regular ConcurrentDictionary

@jeremydmiller
Copy link
Member

@migajek I don't think this code does any harm per se, but can you explain a little bit more what that race condition is? How did you get that?

And ImTools is significantly faster than ConcurrentDictionary, hence it's usage all over Wolverine and Marten

@migajek
Copy link
Author

migajek commented Sep 2, 2025

Understood. I assumed they're still more performant especially since these collections are mostly read.

For the circumstances - you're gonna hate the answer ;)
It's a bunch of Wolverine.Http endpoints, each injects IMessageBus and calls InvokeAsync. The important bit is, each endpoint sends a different message type and expects different response type.
When exposed to a heavy load of http calls immediately after start, occasionally some of the handlers are being called in parallel for the first time.

The call chain:
MessageBus.InvokeAsync -> MessageRoute.InvokeAsync -> MessageRoute.RemoteInvokeAsync -> bus.Runtime.RegisterMessageType(typeof(T));

Obviously the quick remedy for the issue was to register all response types upfront, but unless uprfront registration is a requirement, I'd go with bullet-proofing the HandlerGraph

@jeremydmiller
Copy link
Member

Makes sense, but also:

Just so you and anyone who reads this knows, the Wolverine team most certainly does not recommend building an application like this:

The call chain:
MessageBus.InvokeAsync -> MessageRoute.InvokeAsync -> MessageRoute.RemoteInvokeAsync -> bus.Runtime.RegisterMessageType(typeof(T));

Just don't do that. Way too much indirection.

@migajek
Copy link
Author

migajek commented Sep 2, 2025

Thanks, but to make things clear - we're not doing anything like return bus.InvokeAsync<TResponse>(TRequest)
The handlers are more complex but first they fetch some auxiliary data from other service for further processing. Hope that's not an anti-pattern, too.

@jeremydmiller jeremydmiller changed the base branch from main to 4.0 September 2, 2025 22:39
@jeremydmiller jeremydmiller changed the base branch from 4.0 to main September 2, 2025 22:39
@jeremydmiller
Copy link
Member

@migajek Just potentially slow. Doing request/reply through Wolverine (really through any service bus) is going to incur more latency than an HTTP call would. Probably a little more efficient to go purely async if you can.

Just so you know, I'm going to pull this into main now that's for 5.0. I'll cherry pick this over to 4.0 in the next day or two for whatever the next 4.* bug fix release is

@jeremydmiller jeremydmiller merged commit b527f6d into JasperFx:main Sep 3, 2025
1 check passed
@migajek
Copy link
Author

migajek commented Sep 4, 2025

thank you, both for the advices and the cherry-pick. cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants