Skip to content

Drop built-in atomicity support in favour of OverlayFS#701

Merged
jviotti merged 2 commits intomainfrom
no-atomicity
Mar 4, 2026
Merged

Drop built-in atomicity support in favour of OverlayFS#701
jviotti merged 2 commits intomainfrom
no-atomicity

Conversation

@jviotti
Copy link
Member

@jviotti jviotti commented Mar 4, 2026

Signed-off-by: Juan Cruz Viotti jv@jviotti.com

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 13 files

@augmentcode
Copy link

augmentcode bot commented Mar 4, 2026

🤖 Augment PR Summary

Summary: This PR removes One’s built-in “atomic commit” mechanism for index output generation, shifting responsibility for atomicity to the execution environment (e.g., OverlayFS).

Changes:

  • Drops the staging directory + sentinel strategy and the final atomic directory swap in src/index/index.cc.
  • Writes build artifacts directly into the final output path via sourcemeta::one::Output.
  • Removes commit-related CLI log lines (no more “Hardlinking:” / “Committing:” messages).
  • Moves the <fstream> dependency to src/index/output.h (where std::ofstream is used).
  • Updates CLI golden-output tests to match the new logging behavior.
  • Removes CLI tests that specifically validated staging/atomic-failure/stale-staging behaviors.

Technical Notes: After this change, a failed indexing run can leave the output directory partially updated unless an external atomicity layer (like OverlayFS) is used.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

/////////////////////////////////////////////////////////////////////////////

sourcemeta::one::Output output{staging_path};
sourcemeta::one::Output output{final_output_path};
Copy link

Choose a reason for hiding this comment

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

Now that Output targets final_output_path directly, any failure after this point can leave the real output directory partially updated/created (where previously the atomic swap avoided touching it). If non-OverlayFS runs are still supported, it may be worth documenting/warning about this behavioral change to avoid surprising data loss.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

std::ofstream sentinel_stream(staging_sentinel);
sentinel_stream.close();

PROFILE_END(profiling, "Commit");
Copy link

Choose a reason for hiding this comment

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

PROFILE_END(profiling, "Commit") now appears to measure an empty stage (the commit/swap logic was removed), which can make --time output misleading. Consider removing or renaming this profiling label so timings reflect real work.

Severity: low

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Benchmark Index (community)

Details
Benchmark suite Current: 6144844 Previous: 2cbd5ce Ratio
Add one schema (0 existing) 41 ms 44 ms 0.93
Add one schema (100 existing) 452 ms 1000 ms 0.45
Add one schema (1000 existing) 4566 ms 9431 ms 0.48

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Benchmark Index (enterprise)

Details
Benchmark suite Current: 6144844 Previous: 2cbd5ce Ratio
Add one schema (0 existing) 45 ms 53 ms 0.85
Add one schema (100 existing) 484 ms 1110 ms 0.44
Add one schema (1000 existing) 4576 ms 10658 ms 0.43

This comment was automatically generated by workflow using github-action-benchmark.

@jviotti jviotti merged commit e41cfe1 into main Mar 4, 2026
6 checks passed
@jviotti jviotti deleted the no-atomicity branch March 4, 2026 13:03
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.

1 participant