Skip to content

Commit 89543df

Browse files
andinuxclaude
andcommitted
Revert "fix(network): advance send checkpoint forward-only and keep first send failure"
This reverts commit 88ddeb1. The forward-only guard was wrong. The cloudsync server computes lastOptimisticVersion as the last *continuous* version over applied ranges plus active (pending/running) apply jobs (internal/syncstatus/status.go, ranges.go:LastContinuousVersion). When an apply job fails it leaves the active set and is not in the applied ranges, so its range becomes a gap and lastOptimisticVersion regresses to just before the failed range. That regression is the server's re-send signal: the client must follow lastOptimisticVersion backward and re-stream from there. The forward-only guard suppressed it, turning a recoverable apply failure into a permanently skipped change. Restoring the original behavior (follow lastOptimisticVersion wherever it points) also makes review finding #5 ("checkpoint can regress") a non-issue -- the regression is intended -- and the keep-first failure change moot, since the server returns a single current failure per response rather than distinct per-chunk failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent dfbb879 commit 89543df

1 file changed

Lines changed: 11 additions & 20 deletions

File tree

src/network/network.c

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1616,20 +1616,16 @@ static void network_sync_state_update_from_response(NETWORK_RESULT *res,
16161616
int parsed_gaps_size = json_extract_array_size(res->buffer, res->blen, "gaps");
16171617
if (parsed_gaps_size >= 0) *gaps_size = parsed_gaps_size;
16181618

1619-
// Keep the FIRST failure observed across the chunk stream, not the last: an
1620-
// earlier chunk's failure is the more likely root cause, and a later chunk's
1621-
// response must not silently mask it (each chunk's response can carry a
1622-
// failures.{apply,check} object).
16231619
char *apply_failure = json_extract_failure_stage(res->buffer, res->blen, "apply");
16241620
if (apply_failure) {
1625-
if (*apply_failure_json) cloudsync_memory_free(apply_failure);
1626-
else *apply_failure_json = apply_failure;
1621+
if (*apply_failure_json) cloudsync_memory_free(*apply_failure_json);
1622+
*apply_failure_json = apply_failure;
16271623
}
16281624

16291625
char *check_failure = json_extract_failure_stage(res->buffer, res->blen, "check");
16301626
if (check_failure) {
1631-
if (*check_failure_json) cloudsync_memory_free(check_failure);
1632-
else *check_failure_json = check_failure;
1627+
if (*check_failure_json) cloudsync_memory_free(*check_failure_json);
1628+
*check_failure_json = check_failure;
16331629
}
16341630
}
16351631

@@ -1790,19 +1786,14 @@ int cloudsync_network_send_changes_internal (sqlite3_context *context, int argc,
17901786
}
17911787
if (gaps_size < 0) gaps_size = 0;
17921788

1793-
// Advance the send checkpoint FORWARD ONLY. The server's lastOptimisticVersion
1794-
// can momentarily report a value below our current checkpoint (a stale or
1795-
// out-of-order status response, or a server that is behind this site); writing
1796-
// it back verbatim (the old "!= db_version" guard) would rewind the cursor and
1797-
// re-stream already-sent data, or worse, interact with a later advance to skip
1798-
// a range. Only move the cursor when the target is strictly ahead. When the
1799-
// server returns no optimistic version, fall back to the max watermark we
1800-
// streamed (new_db_version, always >= db_version).
1789+
// update db_version in settings
18011790
char buf[256];
1802-
if (last_optimistic_version > db_version) {
1803-
snprintf(buf, sizeof(buf), "%" PRId64, last_optimistic_version);
1804-
dbutils_settings_set_key_value(data, CLOUDSYNC_KEY_SEND_DBVERSION, buf);
1805-
} else if (last_optimistic_version < 0 && new_db_version > db_version) {
1791+
if (last_optimistic_version >= 0) {
1792+
if (last_optimistic_version != db_version) {
1793+
snprintf(buf, sizeof(buf), "%" PRId64, last_optimistic_version);
1794+
dbutils_settings_set_key_value(data, CLOUDSYNC_KEY_SEND_DBVERSION, buf);
1795+
}
1796+
} else if (new_db_version != db_version) {
18061797
snprintf(buf, sizeof(buf), "%" PRId64, new_db_version);
18071798
dbutils_settings_set_key_value(data, CLOUDSYNC_KEY_SEND_DBVERSION, buf);
18081799
}

0 commit comments

Comments
 (0)