Skip to content

Preserve concurrent page edits during rollback #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jzjzzzzzzz wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Preserve concurrent page edits during rollback #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 66 additions & 7 deletions Tools/project_page_state.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class StagedProjection:
"projection", "parent_fd", "temporary_name", "backup_name",
"stage_artifact_name", "rollback_artifact_name", "stage_fd",
"stage_dev", "stage_ino", "stage_mode", "claimed", "published",
"after_image_verified", "published_size", "published_mtime_ns",
)

def __init__(self, projection, parent_fd, temporary_name, backup_name,
Expand All @@ -84,6 +85,9 @@ def __init__(self, projection, parent_fd, temporary_name, backup_name,
self.stage_mode = stage_descriptor.st_mode
self.claimed = False
self.published = False
self.after_image_verified = False
self.published_size = None
self.published_mtime_ns = None


def _staged_artifact_descriptor(staged):
Expand All @@ -100,6 +104,9 @@ def _staged_artifact_descriptor(staged):
"rollback_image_active": staged.backup_name,
"claimed": staged.claimed,
"published": staged.published,
"after_image_verified": staged.after_image_verified,
"published_size": staged.published_size,
"published_mtime_ns": staged.published_mtime_ns,
}


Expand Down Expand Up @@ -542,9 +549,36 @@ def _publish_staged(root, staged):
raise OSError(errno.EAGAIN,
"installed page differs from staged after-image",
page.relative)
os.unlink(staged.temporary_name, dir_fd=staged.parent_fd)
os.unlink(staged.stage_artifact_name, dir_fd=staged.parent_fd)
# The installed name has been verified once as the exact planned
# after-image and its staging alias is gone. From this point onward a
# byte change can only be a later writer, so rollback must preserve it
# rather than treating it as corruption of an unpublished stage.
staged.temporary_name = None
os.fsync(parent_fd)
verified_after = os.fstat(staged.stage_fd)
named_verified_after = os.stat(
basename, dir_fd=parent_fd, follow_symlinks=False)
if ((verified_after.st_dev, verified_after.st_ino) !=
(staged.stage_dev, staged.stage_ino) or
(named_verified_after.st_dev, named_verified_after.st_ino) !=
(verified_after.st_dev, verified_after.st_ino) or
verified_after.st_nlink != 1 or
_read_exact_descriptor(staged.stage_fd) != page.after_data):
raise OSError(errno.EAGAIN,
"installed page changed after publication",
page.relative)
# Closing the staging link changes ctime once more. Capture the stable
# single-name identity only after that descriptor is closed.
os.close(staged.stage_fd)
staged.stage_fd = None
verified_after = os.stat(
basename, dir_fd=parent_fd, follow_symlinks=False)
staged.published_size = verified_after.st_size
staged.published_mtime_ns = getattr(
verified_after, "st_mtime_ns",
int(verified_after.st_mtime * 1e9))
staged.after_image_verified = True
held_data = _read_exact_descriptor(target_fd)
held = os.fstat(target_fd)
if (held.st_dev, held.st_ino) != (page.snapshot.dev,
Expand All @@ -566,16 +600,28 @@ def _restore_staged(root, staged):
parent_fd, basename = _open_parent(root, staged.projection.relative)
try:
if staged.published:
publication_complete = staged.after_image_verified
current = kblib.repository_target_snapshot(
root, staged.projection.relative, suffixes=".md",
singly_linked=False)
if (not current.exists or
current.dev != staged.stage_dev or
current.ino != staged.stage_ino or
current.mode != staged.stage_mode):
current.mode != staged.stage_mode or
(publication_complete and current.nlink != 1) or
(not publication_complete and current.nlink not in (1, 2))):
raise OSError(errno.EAGAIN,
"published page identity changed before rollback",
staged.projection.relative)
if (publication_complete and
(current.size != staged.published_size or
current.mtime_ns != staged.published_mtime_ns or
current.data != staged.projection.after_data)):
raise OSError(
errno.EAGAIN,
"published page bytes changed before rollback; "
"preserving the concurrent page and recovery evidence",
staged.projection.relative)
named = os.stat(basename, dir_fd=parent_fd,
follow_symlinks=False)
if (named.st_dev, named.st_ino) != (current.dev, current.ino):
Expand All @@ -597,13 +643,26 @@ def _restore_staged(root, staged):
held_after = os.fstat(held_after_fd)
named_after = os.stat(basename, dir_fd=parent_fd,
follow_symlinks=False)
if ((held_after.st_dev, held_after.st_ino) !=
(staged.stage_dev, staged.stage_ino) or
(named_after.st_dev, named_after.st_ino) !=
(held_after.st_dev, held_after.st_ino)):
identity_changed = (
(held_after.st_dev, held_after.st_ino) !=
(staged.stage_dev, staged.stage_ino) or
(named_after.st_dev, named_after.st_ino) !=
(held_after.st_dev, held_after.st_ino) or
held_after.st_mode != staged.stage_mode or
(publication_complete and
held_after.st_size != staged.published_size) or
(publication_complete and getattr(
held_after, "st_mtime_ns",
int(held_after.st_mtime * 1e9)) !=
staged.published_mtime_ns) or
(publication_complete and held_after.st_nlink != 2) or
(not publication_complete and held_after.st_nlink not in
(2, 3))
)
if identity_changed:
raise OSError(
errno.EAGAIN,
"published page changed during rollback claim",
"published page identity changed before rollback",
staged.projection.relative)
os.unlink(basename, dir_fd=parent_fd)
finally:
Expand Down
49 changes: 49 additions & 0 deletions Tools/tests/test_project_page_state.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,55 @@ def fail_second_after_claim(source, destination, *args, **kwargs):
self.assertEqual([], list(pathlib.Path(root).rglob(
".cambium-page-state-*")))

def test_later_failure_does_not_overwrite_concurrent_published_edit(self):
"""Rollback is a CAS: a changed after-image belongs to its writer."""
root = self.build()
page = pathlib.Path(root, "Domain/Closed.md")
concurrent = "Concurrent edit after page-state publication.\n"
original_publish = project_page_state._publish_staged
publications = 0

def edit_first_then_fail_second(root_path, staged):
nonlocal publications
publications += 1
if publications == 1:
result = original_publish(root_path, staged)
# An uncooperative writer can change the installed inode while
# the projector moves on to the next page. A later rollback
# must not erase those bytes merely because the inode still
# belongs to this transaction's published after-image.
page.write_text(concurrent, encoding="utf-8")
return result
raise OSError("injected later-page publication failure")

with mock.patch.object(
project_page_state, "_publish_staged",
side_effect=edit_first_then_fail_second):
code, output = self.run_main(root, "--apply")

self.assertEqual(1, code, output)
self.assertGreaterEqual(publications, 2)
self.assertEqual(concurrent, page.read_text(encoding="utf-8"))
self.assertIn("rollback is incomplete", output)
lock = pathlib.Path(root, ".cambium/tmp/state-writer.lock")
self.assertTrue(lock.is_dir())
owner = json.loads((lock / "owner.json").read_text(encoding="utf-8"))
self.assertEqual("rollback-required",
owner.get("operation", {}).get("status"))
journal = json.loads(
(lock / project_page_state.JOURNAL_NAME).read_text(
encoding="utf-8"))
artifact = next(
row for row in journal.get("artifacts", [])
if row.get("path") == "Domain/Closed.md")
self.assertIsNone(artifact.get("staged_after_image_active"))
self.assertTrue(artifact.get("after_image_verified"))
self.assertTrue(artifact.get("published"))
self.assertTrue(artifact.get("claimed"))
backup = pathlib.Path(root, "Domain", artifact["rollback_image"])
self.assertTrue(backup.is_file())
self.assertEqual(CLOSED.encode("utf-8"), backup.read_bytes())

def test_late_coverage_change_rolls_back_all_pages(self):
"""A stale Ledger owner cannot leave a successful projection."""
root = self.build()
Expand Down
Loading