Skip to content

Commit b7a804d

Browse files
committed
Merge remote-tracking branch 'origin/codex/analyze-project-for-idempotent-command-support'
# Conflicts: # ManagedCode.Communication.Orleans/Grains/CommandIdempotencyGrain.cs # ManagedCode.Communication/Commands/Stores/MemoryCacheCommandIdempotencyStore.cs # README.md
2 parents ff21939 + 2a69bea commit b7a804d

File tree

2 files changed

+50
-28
lines changed

2 files changed

+50
-28
lines changed

ManagedCode.Communication/Commands/Stores/MemoryCacheCommandIdempotencyStore.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,7 @@ private void ReleaseLockReference(string commandId, CommandLock commandLock)
107107
{
108108
if (Interlocked.Decrement(ref commandLock.RefCount) == 0)
109109
{
110-
if (_commandLocks.TryGetValue(commandId, out var existingLock) && ReferenceEquals(existingLock, commandLock))
111-
{
112-
_commandLocks.TryRemove(new KeyValuePair<string, CommandLock>(commandId, commandLock));
113-
}
114-
110+
_commandLocks.TryRemove(new KeyValuePair<string, CommandLock>(commandId, commandLock));
115111
commandLock.Semaphore.Dispose();
116112
}
117113
}

README.md

Lines changed: 49 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -569,59 +569,85 @@ if (status == CommandExecutionStatus.Completed)
569569

570570
### Command Correlation and Tracing Identifiers
571571

572-
Commands implement `ICommand` and surface correlation, causation, trace, span, user, and session identifiers alongside optional metadata so every hop can attach observability context. The base `Command` and `Command<T>` types keep those properties on the root object, and serializers/Orleans surrogates round-trip them without custom plumbing.
572+
Commands implement `ICommand` and surface correlation, causation, trace, span, user, and session identifiers alongside optional metadata so every hop can attach observability context. The base `Command` and `Command<T>` types keep those properties on the
573+
root object, and serializers/Orleans surrogates round-trip them without custom plumbing.
574+
root object, and serializers/Orleans surrogates round-trip them without custom plumbing.
573575

574576
#### Identifier lifecycle
575577
- Static command factories generate monotonic version 7 identifiers via `Guid.CreateVersion7()` and stamp a UTC timestamp so commands can be sorted chronologically even when sharded.
576578
- Factory helpers never mutate the correlation or trace identifiers; callers opt in by supplying values through fluent `WithCorrelationId`, `WithTraceId`, and similar extension methods that return the same command instance.
577-
- Metadata mirrors the trace/span identifiers for workload-specific diagnostics without coupling transport-level identifiers to payload annotations.
579+
- Metadata mirrors the trace/span identifiers for workload-specific diagnostics without coupling transport-level identifiers to
580+
payload annotations.
578581

579582
#### Field reference
580583

581584
| Field | Purpose | Typical source | Notes |
582585
| --- | --- | --- | --- |
583586
| `CommandId` | Unique, monotonic identifier for deduplication | Static command factories | Remains stable for retries and storage lookups. |
584-
| `CorrelationId` | Ties a command to an upstream workflow/request | HTTP `X-Correlation-Id`, message headers | Preserved through serialization and Orleans surrogates. |
587+
| `CorrelationId` | Ties a command to an upstream workflow/request | HTTP `X-Correlation-Id`, message headers | Preserved through
588+
serialization and Orleans surrogates. |
585589
| `CausationId` | Records the predecessor command/event | Current command ID | Supports causal chains in telemetry. |
586-
| `TraceId` | Connects to distributed tracing spans | OpenTelemetry/`Activity` context | The library stores, but never generates, trace identifiers. |
587-
| `SpanId` | Identifies the originating span | OpenTelemetry/`Activity` context | Often paired with `Metadata.TraceId` for deeper traces. |
590+
| `TraceId` | Connects to distributed tracing spans | OpenTelemetry/`Activity` context | The library stores, but never generate
591+
s, trace identifiers. |
592+
| `SpanId` | Identifies the originating span | OpenTelemetry/`Activity` context | Often paired with `Metadata.TraceId` for deep
593+
er traces. |
588594
| `UserId` / `SessionId` | Attach security/session principals | Authentication middleware | Useful for multi-tenant auditing. |
589595

590596
#### Trace vs. correlation
591-
- **Correlation IDs** bundle every command spawned from a single business request. Assign them at ingress and keep the value stable across retries so dashboards can answer “what commands ran because of this call?”.
592-
- **Trace/Span IDs** follow distributed tracing semantics. Commands avoid creating new traces and instead persist the ambient `Activity` identifiers through serialization so telemetry back-ends can stitch spans together.
593-
- Both identifier sets are serialized together, enabling pivots between business-level correlation and technical call graphs without extra configuration.
597+
- **Correlation IDs** bundle every command spawned from a single business request. Assign them at ingress and keep the value st
598+
able across retries so dashboards can answer “what commands ran because of this call?”.
599+
- **Trace/Span IDs** follow distributed tracing semantics. Commands avoid creating new traces and instead persist the ambient `A
600+
ctivity` identifiers through serialization so telemetry back-ends can stitch spans together.
601+
- Both identifier sets are serialized together, enabling pivots between business-level correlation and technical call graphs wit
602+
hout extra configuration.
594603

595604
#### Generation and propagation guidance
596-
- Use `Command.Create(...)` / `Command<T>.Create(...)` (or the matching `From(...)` helpers) to get a version 7 identifier and UTC timestamp automatically.
597-
- Read or generate correlation IDs from HTTP headers or upstream messages and apply them via `.WithCorrelationId(...)` before dispatching commands.
598-
- Capture `Activity.TraceId`/`Activity.SpanId` through `.WithTraceId(...)` and `.WithSpanId(...)` (and metadata counterparts) when bridging to queues, Orleans, or background pipelines.
605+
- Use `Command.Create(...)` / `Command<T>.Create(...)` (or the matching `From(...)` helpers) to get a version 7 identifier and U
606+
TC timestamp automatically.
607+
- Read or generate correlation IDs from HTTP headers or upstream messages and apply them via `.WithCorrelationId(...)` before d
608+
ispatching commands.
609+
- Capture `Activity.TraceId`/`Activity.SpanId` through `.WithTraceId(...)` and `.WithSpanId(...)` (and metadata counterparts) wh
610+
en bridging to queues, Orleans, or background pipelines.
599611
- Serialization tests verify the identifiers round-trip, so consumers can rely on receiving the same values they emitted.
600612

601613
#### Operational considerations
602-
- Factory unit tests ensure commands created through the helpers carry version 7 identifiers, UTC timestamps, and derived `CommandType` values for traceability.
603-
- Idempotency regression tests assert that concurrent callers reuse cached results and propagate failures consistently, preserving correlation integrity when retry storms occur.
614+
- Factory unit tests ensure commands created through the helpers carry version 7 identifiers, UTC timestamps, and derived `Comma
615+
ndType` values for traceability.
616+
- Idempotency regression tests assert that concurrent callers reuse cached results and propagate failures consistently, preservi
617+
ng correlation integrity when retry storms occur.
604618

605619
### Idempotency Architecture Overview
606620

607621
#### Scope
608-
The shared idempotency helpers (`CommandIdempotencyExtensions`), default in-memory store, and test coverage work together to protect concurrency, caching, and retry behaviour across hosts.
622+
The shared idempotency helpers (`CommandIdempotencyExtensions`), default in-memory store, and test coverage work together to pro
623+
tect concurrency, caching, and retry behaviour across hosts.
609624

610625
#### Strengths
611-
- **Deterministic status transitions.** `ExecuteIdempotentAsync` only invokes the provided delegate after atomically claiming the command, writes the result, and then flips the status to `Completed`, so retries either reuse cached output or wait for the in-flight execution to finish.
612-
- **Batch reuse of cached outputs.** Batch helpers perform bulk status/result lookups and bypass execution for already completed commands, even when cached results are `null` or default values.
613-
- **Fine-grained locking in the memory store.** Per-command `SemaphoreSlim` instances eliminate global contention, and reference counting ensures locks are released once no callers use a key.
614-
- **Concurrency regression tests.** Dedicated unit tests confirm that concurrent callers share a single execution, failed primary runs surface consistent exceptions, and the final status ends up in `Failed` when appropriate.
626+
- **Deterministic status transitions.** `ExecuteIdempotentAsync` only invokes the provided delegate after atomically claiming th
627+
e command, writes the result, and then flips the status to `Completed`, so retries either reuse cached output or wait for the in
628+
-flight execution to finish.
629+
- **Batch reuse of cached outputs.** Batch helpers perform bulk status/result lookups and bypass execution for already completed
630+
commands, even when cached results are `null` or default values.
631+
- **Fine-grained locking in the memory store.** Per-command `SemaphoreSlim` instances eliminate global contention, and reference
632+
counting ensures locks are released once no callers use a key.
633+
- **Concurrency regression tests.** Dedicated unit tests confirm that concurrent callers share a single execution, failed primar
634+
y runs surface consistent exceptions, and the final status ends up in `Failed` when appropriate.
615635

616636
#### Risks & considerations
617-
- **Missing-result ambiguity.** If a store reports `Completed` but the result entry expired, the extensions currently return the default value. Stores that can distinguish “missing” from “stored default” should override `TryGetCachedResultAsync` to trigger a re-execution.
618-
- **Wait semantics rely on polling.** Adaptive polling keeps responsiveness reasonable, but distributed stores can swap in push-style notifications if tail latency becomes critical.
619-
- **Status retention policies.** The memory store’s cleanup removes status and result after a TTL; other implementations must provide similar hygiene to avoid unbounded growth while keeping enough history for retries.
637+
- **Missing-result ambiguity.** If a store reports `Completed` but the result entry expired, the extensions currently return the
638+
default value. Stores that can distinguish “missing” from “stored default” should override `TryGetCachedResultAsync` to trigger
639+
a re-execution.
640+
- **Wait semantics rely on polling.** Adaptive polling keeps responsiveness reasonable, but distributed stores can swap in push-
641+
style notifications if tail latency becomes critical.
642+
- **Status retention policies.** The memory store’s cleanup removes status and result after a TTL; other implementations must pr
643+
ovide similar hygiene to avoid unbounded growth while keeping enough history for retries.
620644

621645
#### Recommendations
622646
1. Document store-specific retention guarantees so callers can tune retry windows.
623-
2. Consider extending the store contract with a boolean flag (or sentinel wrapper) that differentiates cached `default` values from missing entries.
624-
3. Monitor lock-pool growth in long-lived applications and log keys that never release to diagnose misbehaving callers before memory pressure builds up.
647+
2. Consider extending the store contract with a boolean flag (or sentinel wrapper) that differentiates cached `default` values f
648+
rom missing entries.
649+
3. Monitor lock-pool growth in long-lived applications and log keys that never release to diagnose misbehaving callers before me
650+
mory pressure builds up.
625651

626652
## Error Handling Patterns
627653

0 commit comments

Comments
 (0)