Skip to content

Exercise MST concurrency safety (#201) - #225

Draft
leynos wants to merge 1 commit into
mainfrom
issue-201-mst-concurrency-property-runs-single-threaded-concurrentunionfind-has-no-concurrent-test
Draft

Exercise MST concurrency safety (#201)#225
leynos wants to merge 1 commit into
mainfrom
issue-201-mst-concurrency-property-runs-single-threaded-concurrentunionfind-has-no-concurrent-test

Conversation

@leynos

@leynos leynos commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Summary

This branch makes MST concurrency coverage exercise real parallel execution,
closing #201. It adds a striped-lock union-find stress test using
std::thread, verifies exact Kruskal output across dedicated one- and
eight-thread Rayon pools, and preserves the CI coverage-ratchet pin.

Closes #201.

The two-pool determinism check completes the relevant Milestone 2 scope from
PR #198.

Review walkthrough

Validation

  • make check-fmt: passed.
  • make lint: passed (Rustdoc, Clippy, and Whitaker).
  • make test: passed (1,101 passed; 1 skipped).
  • make markdownlint: passed (0 errors).
  • make nixie: passed.
  • coderabbit review --agent: completed with zero findings.

References

Summary by Sourcery

Exercise MST concurrency behavior under real parallel execution and verify deterministic results across Rayon thread counts.

Enhancements:

  • Add real concurrent stress coverage for the striped-lock union-find implementation and verify its final partition against a sequential oracle.
  • Verify that parallel Kruskal produces identical forests in dedicated one- and eight-thread Rayon pools.

CI:

  • Document that the CI coverage-ratchet thread pin is for reproducibility and does not replace explicit MST concurrency coverage.

Tests:

  • Extend MST property-based and parameterized testing with thread-pool determinism cases and concurrent union-find stress scenarios.

Exercise the striped-lock union-find from simultaneous worker threads and
compare its final partition with a scalar oracle. Verify that parallel
Kruskal returns the identical forest from dedicated one- and eight-thread
Rayon pools, independent of the coverage-ratchet thread pin.
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Warning

Your free Security trial is over. An organization admin can activate billing to continue.


Comment @coderabbitai help to get the list of available commands.

codescene-access[bot]

This comment was marked as outdated.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No quality gates enabled for this code.

@sourcery-ai

sourcery-ai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR closes the MST concurrency-coverage gap by adding a barrier-synchronized, contended union-find stress test and explicit one-versus-eight-thread Rayon determinism properties, while retaining the CI Rayon pin solely for reproducible coverage-ratchet results.

Sequence diagram for concurrent union-find stress test

sequenceDiagram
    participant Test
    participant Workers as std::thread workers
    participant Barrier
    participant UnionFind as ConcurrentUnionFind
    participant Oracle as Sequential oracle

    Test->>Workers: spawn thread_count workers
    Test->>Barrier: wait()
    Workers->>Barrier: wait()
    Barrier-->>Workers: release simultaneously
    loop assigned edges
        Workers->>UnionFind: try_union(left, right)
        UnionFind-->>Workers: Result<bool, MstError>
    end
    Workers-->>Test: join()
    Test->>UnionFind: root_of(node)
    Test->>Oracle: sequential_oracle(edges)
    Test->>Test: compare normalized labels and components
Loading

Sequence diagram for Rayon thread-pool determinism property

sequenceDiagram
    participant Property
    participant OnePool as Rayon pool: 1 thread
    participant EightPool as Rayon pool: 8 threads
    participant Kruskal as parallel_kruskal

    Property->>OnePool: install(|| parallel_kruskal(...))
    OnePool->>Kruskal: parallel_kruskal(node_count, harvest)
    Kruskal-->>OnePool: one_thread_forest
    OnePool-->>Property: result
    Property->>EightPool: install(|| parallel_kruskal(...))
    EightPool->>Kruskal: parallel_kruskal(node_count, harvest)
    Kruskal-->>EightPool: eight_thread_forest
    EightPool-->>Property: result
    Property->>Property: assert forests are equal
Loading

File-Level Changes

Change Details Files
Added a genuinely parallel striped-lock union-find stress test against a sequential partition oracle.
  • Runs seeded workloads across 2, 4, and 8 std::thread workers synchronized by a barrier.
  • Uses many edges over a small node set to force lock contention and retry interleavings.
  • Validates normalized component labels and component count after all workers join.
chutoro-core/src/mst/union_find.rs
Added deterministic parallel Kruskal coverage that is independent of the global Rayon configuration.
  • Builds isolated Rayon pools with one and eight threads for each fixture.
  • Runs parallel Kruskal against separate edge harvests and compares exact forests.
  • Reports pool-construction, algorithm, and output-divergence failures with fixture context.
chutoro-core/src/mst/property/concurrency.rs
Wired thread-count determinism into generated and fixed property-test coverage.
  • Registers the new proptest runner alongside existing MST properties.
  • Adds parameterized fixture coverage for the one-versus-eight-thread comparison.
  • Updates test-module documentation to reflect the expanded property set.
chutoro-core/src/mst/property/tests.rs
Preserved reproducible coverage-ratchet behavior while documenting the separate concurrency safeguards.
  • Keeps CI pinned to one Rayon worker for stable coverage-ratchet input.
  • Documents that dedicated std::thread and ThreadPoolBuilder tests still exercise real parallelism.
.github/workflows/ci.yml

Assessment against linked issues

Issue Objective Addressed Explanation
#201 Add a genuinely concurrent test for ConcurrentUnionFind::try_union that runs multiple threads on a contended edge stream and verifies the resulting partition and component count against a sequential oracle.
#201 Add a determinism test that executes the same MST fixture in explicit one-thread and eight-thread Rayon pools and asserts exact MinimumSpanningForest equality without mutating environment variables.
#201 Ensure the new concurrency coverage is exercised in tests while retaining the CI RAYON_NUM_THREADS=1 coverage-ratchet pin with an appropriate rationale.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

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.

MST concurrency property runs single-threaded; ConcurrentUnionFind has no concurrent test

1 participant