sillsdev / sillsdev/harmony

[claude] Sync head truncates HybridDateTime to milliseconds, permanently stranding commits

Open
#89 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Data bug
Dominant language
C#
Stars
14
Forks
4
Avg merge
3d 21h
Merged PRs (30d)
3

Description

[Claude, autonomous]

[!TIP]
A starter branch is attached to this issue to jump-start the implementation: claude/harmony-issue-89-oga6ra  •  browse the diff

It takes the boringly-simple route instead of the full-precision one below: make the sync-head comparison inclusive on the millisecond (>>=) in both GetMissingCommits paths, so the head's whole millisecond is re-offered rather than stranded. Per-client the HLC is monotonic, so re-offering that millisecond can never leapfrog a commit. Re-applying a commit the remote already holds is idempotent (FilterExistingCommits drops it), so the only cost is re-sending the head-millisecond commits each sync. No SyncState wire-format change and no back-compat path needed.

Tests cover every flavour of the bug and each fails before the one-line-per-path change: counter-sibling on the head millisecond, a stranded commit after the head advances (both on a whole-millisecond boundary, which the DB pre-filter drops, and with sub-millisecond ticks, which the in-memory guard drops), a full clamp burst, and an end-to-end sync across both syncable backends.

Bug. The per-client sync head is Max(DateTime).ToUnixTimeMilliseconds() (QueryHelpers.GetSyncState), and GetMissingCommits selects the push set with a strict > on that millisecond — both the DB pre-filter and the in-memory guard (QueryHelpers.cs:53,56). But commits carry microsecond DateTime and a Counter. Any commit sharing the head's millisecond fails ms > ms and is treated as already-synced: never offered, and the sync reports success. Once a later commit advances the head, the DB pre-filter excludes it too. The result is permanent, silent commit loss — usually just missing data, or a NotSupportedException in SnapshotWorker if the stranded commit was the create for an entity whose later edit did sync.

How several commits end up sharing one millisecond — HLC clamping. When the wall clock is at or behind lastDateTime, GetDateTime stamps successive commits with the same DateTime and an incrementing Counter. Two ways in: a peer's future-dated commit pulls the HLC ahead of local real time, or the local clock steps back. Either way, every commit made during the catch-up window shares one millisecond; per client, only the first that the server already holds survives, and the rest strand.

Fix. Make the head precision-complete: carry the full HybridDateTime (DateTime + Counter) per client and compare with (DateTime, Counter, Id) ordering, as WhereAfter already does. Per-client commits strictly increase in (DateTime, Counter), so a full-precision watermark is exact. SyncState is a client↔server wire type, so keep a back-compat path for clients still sending millisecond-only heads.

Related: #79 (same watermark weakness, ClientId-duplication variant; its per-client digest would also detect this loss); #7 (future-dated commits, one clamp trigger).

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with QueryHelpers.GetSyncState and both GetMissingCommits paths in QueryHelpers.cs, then compare their ordering with WhereAfter and inspect SyncState. Run the tests covering counter-sibling, stranded commits, clamp bursts, and end-to-end sync across both syncable backends. Done means the described loss cases no longer strand commits and the full test suite passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.