Skip to content

fix: bound Common Crawl orchestration fan-out - #3229

Queued
jebrans wants to merge 1 commit into
mainfrom
dev/jebransyed/bound-commoncrawl-fanout-review
Queued

jebrans wants to merge 1 commit into
mainfrom
dev/jebransyed/bound-commoncrawl-fanout-review

Conversation

@jebrans

@jebrans jebrans commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Share bounded crawl-parameter validation across the HTTP starter, direct orchestration entry point, and initial index activity. Reject invalid HTTP requests with JSON 400 before creating a Durable client or starting a job.
  • Enforce the requested index-record count locally and defensively in the orchestrator. Use small, non-overlapping windows for both processing and batch saving, with a finite total activity-call budget.
  • Preserve the existing activity names, result shapes, defaults, batch ordering, JSON/CSV storage pipeline, and PR fix: bound Common Crawl record downloads and decompression #3200's per-record extraction protections, including partial and empty batches.
  • Use recorded Durable time and bundled suffix data for replay-safe naming. Document rollout constraints, residual limits, and the conservative per-worker concurrency settings.

Why this is needed

Previously, a crawl requesting two records could process 200 if the index returned 200. With a batch size of one, it scheduled 200 processing activities before waiting, then 200 batch saves before waiting. This change rejects that excess before processing.

Similarly, batch_size=0 previously started a workflow and failed later. It now returns a useful validation error without starting any orchestration.

Policy and compatibility

Setting Starting policy
Requested records Default 1,000; accepted range 1-1,000
Batch size Default 50; accepted range 1-50
Outstanding activities At most four per orchestration, including queued tasks
Total logical activity calls At most 2 * ceil(N / B) + 2, up to 2,002
Excess index records Explicit error; no silent truncation
Initial-index timeouts Five-second connect / 30-second read inactivity timeout
Host concurrency One loaded activity and one loaded orchestrator per worker initially

The default crawl has 20 batches and at most 42 logical activity calls. These are count/scheduling limits, not measured worker-capacity figures or exactly-once execution guarantees.

Missing/empty numeric HTTP parameters retain their defaults. Direct inputs retain missing-field defaults but reject explicit invalid values. Existing successful starter status responses and output formats are unchanged. Direct orchestration validation/contract errors return an error payload; the Durable instance may therefore be Completed with status: error, not Failed. Activity exceptions still propagate.

Requests, urllib3, and warcio pins from #3200 are unchanged. Add azure-functions-durable<2 to retain this example's existing function.json programming model; no SDK-model migration is included.

Validation

  • 77 offline Python tests passed on Python 3.11 with resource warnings treated as errors: 46 existing extraction tests plus 31 new tests.
  • Tests cover exact boundaries, malformed inputs, zero scheduling on rejection, streaming/closure/error handling, maximum outstanding/total calls, partial/empty extraction, storage/result contracts, and replay-safe naming.
  • The real Durable SDK entry-point test replays recorded history with reversed task completion, a partial final batch, and assertions for saved-result and combined-file ordering.
  • compileall, pip check, repository-pinned Prettier checks for the README/host config, and git diff --check passed.
  • Repository policy checks passed. Existing TypeScript lint, complexity, test-debt, and changed-file formatting selectors report no changed TypeScript source. The full circular-dependency ratchet passed with no new cycles.
  • Validated with Azure Functions 1.25.0, Durable 1.7.0, Requests 2.34.2, urllib3 2.8.0, warcio 1.8.1, and tldextract 5.4.0.

Tests use mocks and recorded/synthetic data, not live Azure, Common Crawl, storage, or secrets. Azure Functions Core Tools are unavailable, so no local Functions-host or deployment test is claimed.

Rollout and limits

Changing task order changes orchestration history. Stop new starts and drain active instances on the old code before deployment; otherwise use a separately versioned deployment/task hub.

Count limits do not impose byte ceilings on initial-index lines, extracted objects, or workflow history, and they are not a global admission/rate limit. Preserve #3200's independent byte limits and verify production worker sizing. The existing logged non-200 index response still becomes an empty result; changing that legacy error/no-data behavior is outside this fix. Do not increase worker activity concurrency without reviewing the save activity's existing shared container state.

Validate crawl parameters and index results, bound processing and save windows, and preserve replay-safe output and extraction contracts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a837848b-dfcf-4f51-962d-90514abd0c99
@jebrans

jebrans commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Independent Gemini review and verification

Reviewed head: 5077c370e291524a8ef509b811aa9a6cff358a4e, against edf9e0e7ebb95f4f779074c6301906042911ca83.

After this PR was created, an independent Gemini 4 Argon subagent reviewed the published diff for correctness, regressions, security concerns, edge cases, missing tests, and unnecessary complexity. It reported no actionable findings and no optional changes. This is an AI review result, not a substitute for required human approval or a guarantee that every possible defect has been excluded.

The result was checked independently rather than accepted solely on the model's authority:

  • Before publication, the author review strengthened actual SDK replay coverage to use reversed activity-completion order, nine records in five batches with a partial final batch, saved-result/blob ordering assertions, and a finite replay-progress check.
  • After the Gemini report, four focused tests were rerun on the exact published head: HTTP rejection before client creation, streamed-index overflow/closure, worst-case activity counts and outstanding windows, and the real SDK replay/storage-order scenario. All four passed.
  • The complete 77-test offline suite, compilation, dependency consistency, explicit formatting, repository policies, and applicable local ratchets passed. The full circular ratchet remained at 210 cycles with no additions.
  • All 25 published CI checks passed for this head, including CodeQL, repository policy, build matrices, and smoke-test checks.

No additional implementation changes were warranted by the Gemini feedback, so no speculative refactor or review-only code churn was added. PR #3200's extraction implementation/tests and the existing save activity/bindings remain unchanged.

The rollout and residual limits in the PR description still apply: drain or version active orchestrations before deployment, verify operational worker sizing, and do not treat count limits as a global rate or memory-byte ceiling. No live Azure deployment or local Functions-host test was performed.

@jebrans
jebrans added this pull request to the merge queue Oct 26, 2026
Any commits made after this event will not be merged.
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 26, 2026
@jebrans
jebrans added this pull request to the merge queue Oct 26, 2026
Any commits made after this event will not be merged.

This branch has not been deployed

No deployments
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