Repository navigation
Mark patches committed without exact values; read only unmarked merges as patchflow 0.8 wrote them - #19
Conversation
…s as patchflow 0.8 wrote them Found by the Markdown meeting test on 0.11.0 (10 users, 15 min): 40 inconsistencies and words lost by merging. Sessions that reloaded during heavy concurrency committed without exact values (no hash) for a while; their merge commits looked like patchflow 0.8 merges and were read by replaying the history in time order, whose result depends on how much of the history a client has loaded. Clients disagreed about those values, and a merge commit recording its author's (wrong) merged value made it everyone's, deleting 8 words in one merge. Session now marks a patch committed without the exact value of its parents `inexact` (it has no hash either). Such a merge commit is read with merge3, like any merge commit of this version, which does not depend on the loaded history. Only unmarked merge commits without a hash or recorded value (written by patchflow 0.8) are replayed in time order. Test: a session that cannot compute the exact merge commits an inexact merge, read with merge3 (the time-order replay gives a different value; the test fails when the marker is ignored). Tests that simulate patchflow 0.8 writers strip the marker, as 0.8 never wrote it.
|
Verified in the browser: the Markdown meeting test (10 users, seed 75) on 0.11.0 had 40 inconsistencies and 10 words lost by merging; with this PR (and CoCalc storing the marker), 0 inconsistencies. With CoCalc's fix for clients that loaded from a document's first snapshot (they never loaded older history, so committed without exact values), the same run has 0 inexact patches and 0 sent words lost (1,818 patches, all hashed but the initial file load; 442 merge commits, all recorded). |
Review of #19 (P2): a patch was marked inexact whenever its parents had no exact value, which is also always the case for a session that never computes exact values (a codec without merge3, or the apply-all strategy). Its merge commits are genuine time-order history, but the marker made exact readers read them with merge3, changing their values. Now only a session that computes exact values (PatchGraph.computesExactValues) and lacks them for a commit marks it. Test: the reviewer's case, a current session without merge3 writing a concurrent history, read unchanged by exact readers (merge3 and a different merge): every value is the author's, no patch is marked. The tests that simulated patchflow 0.8 writers no longer strip the marker (fails with the previous rule).
|
Review P2 fixed in 5ed0196: only a session that computes exact values (codec with merge3, not apply-all) marks a patch inexact when it lacks the exact value of its parents; a session without merge3 writes unmarked time-order history, read as before. Added the reviewer's case as a test (current no-merge3 writer → exact readers, envelopes unchanged: values equal the author's; fails with the previous rule), and the 0.8-writer simulations no longer strip the marker (so the opt-in mixed-version test emits real time-order envelopes again). 157 tests pass; lint/build clean. Release PR #20 rebased on it. |
1 participant
Found by the Markdown meeting test on 0.11.0 (10 users, 15 min): 40
inconsistencies and words lost by merging. Sessions that reloaded during
heavy concurrency committed without exact values (no hash) for a while; their
merge commits looked like patchflow 0.8 merges and were read by replaying the
history in time order, whose result depends on how much of the history a
client has loaded. Clients disagreed about those values, and a merge commit
recording its author's (wrong) merged value made it everyone's, deleting 8
words in one merge.
Session now marks a patch committed without the exact value of its parents
inexact(it has no hash either). Such a merge commit is read with merge3,like any merge commit of this version, which does not depend on the loaded
history. Only unmarked merge commits without a hash or recorded value (written
by patchflow 0.8) are replayed in time order.
Test: a session that cannot compute the exact merge commits an inexact merge,
read with merge3 (the time-order replay gives a different value; the test
fails when the marker is ignored). Tests that simulate patchflow 0.8 writers
strip the marker, as 0.8 never wrote it.