Skip to content

Fix continuous history export skipping out-of-order completions - #830

Open
Bernd Verst (berndverst) wants to merge 2 commits into
mainfrom
fix-export-history-completion-windows
Open

Bernd Verst (berndverst) wants to merge 2 commits into
mainfrom
fix-export-history-completion-windows

Conversation

@berndverst

Copy link
Copy Markdown
Member

Summary

What changed?

  • Replace the lifetime creation-order cursor used by continuous export with pagination scoped to fixed completion-time windows.
  • Extend ExportCheckpoint with nullable CompletedTimeFrom and CompletedTimeTo properties. Keep both bounds fixed while paging; preserve them alongside the cursor across entity checkpoints and continue-as-new.
  • After exhausting a window, reset the creation-order cursor, advance the lower completion-time bound to the previous upper bound, and wait for the next poll.
  • Handle empty pages with advancing cursors by continuing to page the same window; treat a missing cursor or an empty page with an unchanged cursor as the end of the window.
  • Gate the new orchestration behavior with ExportJobRunRequest.UseCompletionTimeWindows. Newly scheduled runners enable it immediately; legacy inputs default to false and adopt it at their next continue-as-new boundary.
  • Reconcile legacy key-only checkpoints by starting from the configured completedTimeFrom instead of retaining a cursor that may already have skipped instances.
  • Add regression coverage and document migration, inclusive boundaries, idempotent writes, and counter semantics.

Why is this change needed?

  • DTS lists terminal instance IDs in creation-key order, not completion-time order. Previously, exporting a later-created instance B permanently moved the cursor past an earlier-created instance A that was still running. When A subsequently completed, it remained behind the cursor and was silently omitted.
  • Bounding each traversal by a fixed completion-time upper bound separates pagination from future completions. In the reported scenario, B belongs to the first window; A completes after that window's upper bound and is found in a subsequent window, whose creation-order cursor starts from the beginning.
  • Merely resetting the cursor against the original unbounded range would repeatedly export all previously completed instances. The persisted completion-time watermark avoids that full-range rescan during normal operation.

Example

Instance Creation key Completion First window Next window
A 1 After the first poll's upper bound Excluded by completion time Found after cursor reset
B 2 Before the first poll's upper bound Exported Outside the next window unless on its boundary

Issues / work items


Project checklist

  • Release notes are not required for the next release
    • Otherwise: Notes added to release_notes.md
  • Backport is not required
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • All required tests have been added/updated (unit tests, E2E tests)
  • Breaking change?
    • If yes:
      • Impact: Intentional behavioral change for continuous export after the replay-safe adoption boundary. Legacy checkpoints are rescanned from the original filter, which can repeat writes and increase export-operation counters. No constructor signatures, existing properties, enum values, protobuf fields, or serializer defaults are removed or changed.
      • Migration guidance: Upgrade the export worker. Existing active runners retain legacy behavior for their current history and enable completion-time windows at the next continue-as-new. Retain source histories until the reconciliation scan finishes; already-purged histories cannot be recovered. Repeated writes use existing deterministic blob names. See the sample README and the compatibility notes below.

Release-note and backport decisions are left to maintainers. Automated SDK regression coverage is included; a live DTS end-to-end test was not run.


AI-assisted code disclosure (required)

Was an AI tool used? (select one)

  • No
  • Yes, AI helped write parts of this PR (e.g., GitHub Copilot)
  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): GitHub Copilot App.
  • AI-assisted areas/files: src/ExportHistory/Models/ExportCheckpoint.cs, src/ExportHistory/Orchestrations/ExportJobOrchestrator.cs, src/ExportHistory/Entity/ExportJob.cs, the corresponding checkpoint/entity/orchestration tests, and samples/ExportHistoryWebApp/README.md.
  • What you changed after AI output: No independent human code edits are claimed. The agent iterated on the tests, validated the implementation, and identified the backend-consistency caveat below.

AI verification (required if AI was used):

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

Checked verification items describe agent verification; the human understanding/approval item remains unchecked for the author/reviewer.


Testing

Automated tests

  • Result: Passed.
  • dotnet test test\ExportHistory.Tests\ExportHistory.Tests.csproj --no-restore --verbosity quiet — passed, 163 tests, 0 failed, 0 skipped.
  • dotnet build src\ExportHistory\ExportHistory.csproj --no-restore --verbosity quiet -clp:ErrorsOnly — passed for the project's net6.0, net8.0, and net10.0 targets. Latest incremental run: 0 warnings, 0 errors.
  • git --no-pager diff --check — passed.

Added coverage includes:

  • The reported A/B out-of-order completion scenario, using the actual listing activity with a mocked creation-order backend and a page size of one.
  • Recovery from a legacy checkpoint already beyond A's creation key.
  • Serialization of legacy checkpoint and runner inputs, and round-tripping completion-window checkpoints.
  • Window/cursor persistence across continue-as-new.
  • End-of-window responses with null, empty, or unchanged tokens.
  • Empty pages whose cursor advances.
  • Inclusive boundary overlap.
  • Failed exports preserving the prior checkpoint.
  • Future lower bounds waiting without listing.
  • Preserved batch and legacy-runner listing behavior.
  • Newly scheduled runners opting into completion-time windows.

Manual validation (only if runtime/behavior changed)

  • Environment (OS, .NET version, components): Windows; .NET SDK 10.0.403; local SDK build and mocked Durable Task client/entity/context components.
  • Steps + observed results:
    1. Model A with creation key 1 completing after the initial window, and B with creation key 2 completing before it. Run the production orchestrator and listing activity against the mocked listing API.
    2. Observe B exported in the first window and A exported after the next window resets the creation-order cursor; verify frozen upper bounds while paging.
    3. Repeat with a legacy checkpoint at B's key, and verify reconciliation starts without that key and exports both instances.
  • Evidence (optional): These scenarios are executable unit regressions, not live-service manual validation. No Azure resources were deployed and no live DTS or Blob Storage end-to-end run was performed.

Notes for reviewers

  • Backend consistency/timestamp guarantee remains an important assumption. The upper bound is context.CurrentUtcDateTime: replay-safe, but not a server-issued snapshot watermark. If an instance becomes query-visible only after its window has been scanned, with a completion timestamp older than the advanced lower bound, it can still be omitted. Worker/service clock skew can create a similar situation. The public listing/protobuf contract does not establish a stable snapshot or bounded visibility delay, and the mocked regression assumes immediate visibility. Please confirm DTS guarantees before treating this as unconditional lossless export.
  • A safety lag or trailing overlap could mitigate bounded delay/skew, but cannot prove completeness for arbitrarily delayed visibility. A server-provided completion-ordered cursor or snapshot watermark would provide a stronger contract.
  • No existing exactly-once guarantee is removed: the previous implementation retried entire partially failed batches, including already successful exports, and blob uploads were not atomic with checkpoint commits. Activity redelivery could also repeat writes. This change adds inclusive-boundary and migration re-exports: instances exactly on a window boundary can be exported twice, and legacy reconciliation can revisit existing exports. Existing deterministic blob naming makes repeated writes idempotent; ExportedInstances counts successful operations, not unique blobs.
  • Migration cost: the first window for a legacy checkpoint rescans the original configured range. This intentionally recovers available skipped histories, but can cause substantial listing/export traffic for long-lived jobs.
  • Replay compatibility: existing inputs deserialize with UseCompletionTimeWindows = false, retaining their current sequence of activity/entity/timer calls. The next continue-as-new supplies true; persisted window checkpoints then survive later restarts. No nondeterministic clock or I/O was introduced into the new orchestration path.
  • API/serialization compatibility: the existing positional record constructors and deconstruction shapes remain intact. The additions are init-only properties with false/null legacy defaults. No protobuf changes or default converter changes are involved.
  • Batch export is not switched to the new traversal logic. Existing retry/failure behavior and blob naming are unchanged.

Breaking Change

Type: behavioral
Impact: Continuous export adopts completion-time-window traversal and legacy-checkpoint reconciliation. Reconciliation and inclusive-boundary overlap can repeat export operations and increase counters while preserving idempotent blob destinations.
Migration: Upgrade the worker, allow existing jobs to reach their next continue-as-new boundary, and retain source histories during reconciliation. Consumers must not interpret export-operation counters as a count of distinct destination blobs.

Scope creation-order pagination to persistent completion-time windows. Adopt the new traversal at a replay-safe continue-as-new boundary and reconcile legacy checkpoints from their original completion filter.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 1, 2026 07:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The README overstates recovery of out-of-order completions and needs to document export gaps caused by delayed visibility or clock skew, advising users to retain source histories and reconcile exports before purging.

1 open issue
What changed in this PR

Addresses #827 by replacing continuous history export’s lifetime creation-order cursor with fixed completion-time windows, allowing later completions of earlier-created instances to be found.

Changes:

  • Persist completion-window bounds alongside pagination checkpoints.
  • Enable the new traversal for new runners and migrate existing runners at continue-as-new boundaries.
  • Add regression coverage and document migration, repeated writes, and counter semantics.
File Description
test/​ExportHistory.Tests/​Orchestrations/​ExportJobOrchestratorTests.cs Tests out-of-order completions, window pagination, recovery, and legacy behavior.
test/​ExportHistory.Tests/​Models/​ExportCheckpointTests.cs Tests legacy deserialization and window-checkpoint round trips.
test/​ExportHistory.Tests/​Entity/​ExportJobTests.cs Verifies new runners enable completion-time windows.
src/​ExportHistory/​Orchestrations/​ExportJobOrchestrator.cs Implements window traversal and replay-safe adoption.
src/​ExportHistory/​Models/​ExportCheckpoint.cs Adds nullable completion-window bounds.
src/​ExportHistory/​Entity/​ExportJob.cs Enables window traversal when scheduling runners.
samples/​ExportHistoryWebApp/​README.md Documents continuous traversal, migration, and repeated-export semantics.

Review effort: Balanced

Comment thread samples/ExportHistoryWebApp/README.md Resolved
Use null-conditional checkpoint key access and document potential export gaps from delayed query visibility or clock skew, including reconciliation before purging.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Traversal completeness relies on DTS visibility and completion-timestamp guarantees not established by public contracts, requiring maintainer confirmation and live-service validation beyond mocked regressions.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

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.

ExportHistory: continuous export silently skips an instance that completes after a later-created instance has been exported

2 participants