Safe message handler sample #73
Conversation
There was a problem hiding this comment.
Added this abstraction as part of this PR so we can reuse dev servers across tests
cretz
force-pushed
the
atomic-message-handlers
branch
from
4d2502b to
be233c9
Compare
cretz
force-pushed
the
atomic-message-handlers
branch
from
a466055 to
3f773a5
Compare
| ExtraArgs = | ||
| [ | ||
| "--dynamic-config-value", | ||
| "frontend.enableUpdateWorkflowExecution=true", |
There was a problem hiding this comment.
I wonder if we should enable stuff like this when go to public preview?
There was a problem hiding this comment.
Yes, I think we should according to the implied opt-in vs opt-out nature of release stages at https://docs.temporal.io/evaluate/release-stages. But of course that is a server thing and not related to this PR.
|
By request, I am not merging this until temporalio/samples-python#123 is merged. |
| # Safe Message Handlers | ||
|
|
||
| This sample shows a workflow using `Temporalio.Workflows.Semaphore` to atomically process certain blocks of workflow | ||
| code to prevent data race issues. The sample code demonstrates assigning cluster nodes to jobs atomically. |
There was a problem hiding this comment.
For some strict definition of data race, that can't happen in Temporal regardless (what's being prevented here is I think more strictly a logical race). I'd suggest we say "to serialize execution of certain blocks of workflow code" just to be extra clear here.
| // This await would be dangerous without nodesLock because it yields control and allows | ||
| // interleaving |
There was a problem hiding this comment.
| // This await would be dangerous without nodesLock because it yields control and allows | |
| // interleaving | |
| // This await would be dangerous without nodesLock because it yields control and would | |
| // allow execution to interleave with other concurrent updates |
|
This needs to be updated to remove all concept of health check and bad nodes (see temporalio/samples-python#132). We also need to make sure we don't continue as new while a handler is running, so we'll need the new all-finished property which means this is likely held up until a .NET SDK release. |
|
Merging now without all handler stuff to not wait on next release. We can come back and add later if we want. |
What was changed
Sample showing atomic message handling in .NET w/
Semaphoreusing the logic from temporalio/samples-python#123