diff --git a/.claude/commands/stress-test-sync-sqlitecloud.md b/.claude/commands/stress-test-sync-sqlitecloud.md index 4bb41fa..22102cb 100644 --- a/.claude/commands/stress-test-sync-sqlitecloud.md +++ b/.claude/commands/stress-test-sync-sqlitecloud.md @@ -106,11 +106,16 @@ Create a bash script at `/tmp/stress_test_concurrent.sh` that: 2. **Defines a worker function** that runs in a subshell for each database: - Each worker logs all output to `/tmp/sync_concurrent_.log` - Each iteration does: - a. **DELETE all rows** → `cloudsync_network_sync(100, 10)` - b. **INSERT rows** (in a single BEGIN/COMMIT transaction) → `cloudsync_network_sync(100, 10)` - c. **UPDATE all rows** → `cloudsync_network_sync(100, 10)` - - Each session must: `.load` the extension, call `cloudsync_network_init()`, `cloudsync_network_set_token()` (if RLS), do the work, call `cloudsync_terminate()` - - Include labeled output lines like `[DB][iter ] deleted/inserted/updated, count=` for grep-ability + a. **UPDATE all/some rows** (e.g., `UPDATE SET value = value + 1;`) + b. **DELETE a few rows** (e.g., `DELETE FROM
WHERE rowid IN (SELECT rowid FROM
ORDER BY RANDOM() LIMIT 10);`) + c. **Sync using the 3-step send/check/check pattern:** + 1. `SELECT cloudsync_network_send_changes();` — send local changes to the server + 2. `SELECT cloudsync_network_check_changes();` — ask the server to prepare a payload of remote changes + 3. Sleep 1 second (outside sqlite3, between two separate sqlite3 invocations) + 4. `SELECT cloudsync_network_check_changes();` — download the prepared payload, if any + - Each sqlite3 session must: `.load` the extension, call `cloudsync_network_init()`/`cloudsync_network_init_custom()`, `cloudsync_network_set_apikey()`/`cloudsync_network_set_token()` (depending on RLS mode), do the work, call `cloudsync_terminate()` + - **Timing**: Log the wall-clock execution time (in milliseconds) for each `cloudsync_network_send_changes()`, `cloudsync_network_check_changes()` call. Use bash `date +%s%3N` before and after each sqlite3 invocation that calls a network function, and compute the delta. Log lines like: `[DB][iter ] send_changes: 123ms`, `[DB][iter ] check_changes_1: 45ms`, `[DB][iter ] check_changes_2: 67ms` + - Include labeled output lines like `[DB][iter ] updated count=, deleted count=` for grep-ability 3. **Launches all workers in parallel** using `&` and collects PIDs @@ -124,9 +129,11 @@ Create a bash script at `/tmp/stress_test_concurrent.sh` that: 6. **Prints final verdict**: PASS (0 errors) or FAIL (errors detected) **Important script details:** -- Use `echo -e` to pipe generated INSERT SQL (with `\n` separators) into sqlite3 -- Row IDs should be unique across databases and iterations: `db_r_` +- Use `echo -e` to pipe generated SQL (with `\n` separators) into sqlite3 +- During database initialization (Step 1), insert `ROWS` initial rows per database in a single transaction so each DB starts with data to update/delete. Row IDs should be unique across databases: `db_r` - User IDs for rows must match the token's userId for RLS to work +- The sync pattern requires **separate sqlite3 invocations** for send_changes and each check_changes call (with a 1-second sleep between the two check_changes calls), so that timing can be measured per-call from bash +- **stderr capture**: All sqlite3 invocations must redirect both stdout and stderr to the log file. Use `>> "$LOG" 2>&1` (in this order — stdout redirect first, then stderr to stdout). For timed calls that capture output in a variable, redirect stderr to the log file separately: `RESULT=$(echo -e "$SQL" | $SQLITE3 "$DB" 2>> "$LOG")` and then echo `$RESULT` to the log as well. This ensures "Runtime error" messages from sqlite3 are never lost. - Use `/bin/bash` (not `/bin/sh`) for arrays and process management Run the script with a 10-minute timeout. @@ -140,13 +147,25 @@ After the test completes, provide a detailed breakdown: 3. **Timeline analysis**: do errors cluster at specific iterations or spread evenly? 4. **Read full log files** if errors are found — show the first and last 30 lines of each log with errors -### Step 7: Optional — Verify Data Integrity +### Step 7: Final Sync and Data Integrity Verification -If the test passes (or even if some errors occurred), verify the final state: +After all workers have terminated, perform a **final sync on every local database** to ensure all databases converge to the same state. Then verify data integrity. -1. Check each local SQLite database for row count -2. Check SQLiteCloud (as admin) for total row count -3. If RLS is enabled, verify no cross-user data leakage +1. **Final sync loop** (max 10 retries): Repeat the following until all local databases have the same row count, or the retry limit is reached: + a. For each local database (sequentially): + - Load the extension, call `cloudsync_network_init`/`cloudsync_network_init_custom`, authenticate with `cloudsync_network_set_apikey`/`cloudsync_network_set_token` + - Run `SELECT cloudsync_network_sync(100, 10);` to sync remaining changes + - Call `cloudsync_terminate()` + b. After syncing all databases, query `SELECT COUNT(*) FROM
` on each database + c. If all row counts are identical, convergence is achieved — break out of the loop + d. Otherwise, log the round number and the distinct row counts, then repeat from (a) + e. If the retry limit is reached without convergence, report it as a failure + +2. **Row count verification**: Report the final row counts. All databases should have the same number of rows. Also check SQLiteCloud (as admin) for total row count. + +3. **Row content verification**: Pick one random row ID from the first database (`SELECT id FROM
ORDER BY RANDOM() LIMIT 1;`). Then query that same row (`SELECT id, user_id, name, value FROM
WHERE id = '';`) on **every** local database. Compare the results — all databases must return identical column values for that row. Report the row ID, the expected values, and any mismatches. + +4. If RLS is enabled, verify no cross-user data leakage. ## Output Format @@ -157,8 +176,8 @@ Report the test results including: | Concurrent databases | N | | Rows per iteration | ROWS | | Iterations per database | ITERATIONS | -| Total CRUD operations | N × ITERATIONS × (DELETE_ALL + ROWS inserts + ROWS updates) | -| Total sync operations | N × ITERATIONS × 6 (3 sends + 3 checks) | +| Total CRUD operations | N × ITERATIONS × (UPDATE_ALL + DELETE_FEW) | +| Total sync operations | N × ITERATIONS × 3 (1 send_changes + 2 check_changes) | | Duration | start to finish time | | Total errors | count | | Error types | categorized list | @@ -175,13 +194,15 @@ If errors are found, include: The test **PASSES** if: 1. All workers complete all iterations 2. Zero `error`, `locked`, `SQLITE_BUSY`, or HTTP 500 responses in any log -3. Final row counts are consistent +3. After the final sync, all local databases have the same row count +4. A randomly selected row has identical content across all local databases The test **FAILS** if: 1. Any worker crashes or fails to complete 2. Any `database is locked` or `SQLITE_BUSY` errors appear 3. Server returns 500 errors under concurrent load -4. Data corruption or inconsistent row counts +4. Row counts differ across local databases after the final sync loop exhausts all retries +5. Row content differs across local databases (data corruption) ## Important Notes diff --git a/docs/postgresql/SUPABASE_FLYIO.md b/docs/postgresql/SUPABASE_FLYIO.md new file mode 100644 index 0000000..5fe8eb1 --- /dev/null +++ b/docs/postgresql/SUPABASE_FLYIO.md @@ -0,0 +1,89 @@ +# Deploying CloudSync to Self-Hosted Supabase on Fly.io + +## Overview + +Build a custom Supabase Postgres image with CloudSync baked in, push it to a container registry, and configure your Fly.io Supabase deployment to use it. + +## Step-by-step + +### 1. Build the custom Supabase Postgres image + +The project includes `docker/postgresql/Dockerfile.supabase` which builds CloudSync into the Supabase Postgres base image. Match the tag to the PG version your Fly.io Supabase uses: + +```bash +# Build with the default Supabase Postgres tag (17.6.1.071) +make postgres-supabase-build + +# Or specify the exact tag your Fly deployment uses: +make postgres-supabase-build SUPABASE_POSTGRES_TAG=17.6.1.071 +``` + +This produces a Docker image tagged as `public.ecr.aws/supabase/postgres:` locally. + +### 2. Tag and push to a container registry + +You need a registry accessible from Fly.io (Docker Hub, GitHub Container Registry, or Fly's own registry): + +```bash +# Tag for your registry +docker tag public.ecr.aws/supabase/postgres:17.6.1.071 \ + registry.fly.io//postgres-cloudsync:17.6.1.071 + +# Push +docker push registry.fly.io//postgres-cloudsync:17.6.1.071 +``` + +Or use Docker Hub / GHCR: + +```bash +docker tag public.ecr.aws/supabase/postgres:17.6.1.071 \ + ghcr.io//supabase-postgres-cloudsync:17.6.1.071 +docker push ghcr.io//supabase-postgres-cloudsync:17.6.1.071 +``` + +### 3. Update your Fly.io Supabase deployment + +In your Fly.io Supabase config (`fly.toml` or however you deployed the DB service), point the Postgres image to your custom image: + +```toml +[build] + image = "ghcr.io//supabase-postgres-cloudsync:17.6.1.071" +``` + +Then redeploy: + +```bash +fly deploy --app +``` + +### 4. Enable the extension + +Connect to your Fly Postgres instance and enable CloudSync: + +```bash +fly postgres connect --app +``` + +```sql +CREATE EXTENSION cloudsync; +SELECT cloudsync_version(); + +-- Initialize sync on a table +SELECT cloudsync_init('my_table'); +``` + +### 5. If using supabase-docker (docker-compose) + +If your Fly.io Supabase is based on the [supabase/supabase](https://github.com/supabase/supabase) docker-compose setup, update the `db` service image in `docker-compose.yml`: + +```yaml +services: + db: + image: ghcr.io//supabase-postgres-cloudsync:17.6.1.071 +``` + +## Important notes + +- **Match the Postgres major version** — the Dockerfile defaults to PG 17 (`SUPABASE_POSTGRES_TAG=17.6.1.071`). Check what your Fly deployment runs with `SHOW server_version;`. +- **ARM vs x86** — if your Fly machines are ARM (`fly.toml` with `vm.size` using arm), build the image for `linux/arm64`: `docker buildx build --platform linux/arm64 ...` +- **RLS considerations** — when using Supabase Auth with Row-Level Security, use a JWT `token` (not `apikey`) when calling sync functions. diff --git a/plans/BATCH_MERGE_AND_RLS.md b/plans/BATCH_MERGE_AND_RLS.md deleted file mode 100644 index def727e..0000000 --- a/plans/BATCH_MERGE_AND_RLS.md +++ /dev/null @@ -1,166 +0,0 @@ -# Deferred Column-Batch Merge and RLS Support - -## Problem - -CloudSync resolves CRDT conflicts per-column, so `cloudsync_payload_apply` processes column changes one at a time. Previously each winning column was written immediately via a single-column `INSERT ... ON CONFLICT DO UPDATE`. This caused two issues with PostgreSQL RLS: - -1. **Partial-column UPSERT fails INSERT WITH CHECK**: An update to just `title` generates `INSERT INTO docs (id, title) VALUES (...) ON CONFLICT DO UPDATE SET title=...`. PostgreSQL evaluates the INSERT `WITH CHECK` policy *before* checking for conflicts. Missing columns (e.g. `user_id`) default to NULL, so `auth.uid() = user_id` fails. The ON CONFLICT path is never reached. - -2. **Premature flush in SPI**: `database_in_transaction()` always returns true inside PostgreSQL SPI. The old code only updated `last_payload_db_version` inside `if (!in_transaction && db_version_changed)`, so the variable stayed at -1, `db_version_changed` was true on every row, and batches flushed after every single column. - -## Solution - -### Batch merge (`merge_pending_batch`) - -New structs in `cloudsync.c`: - -- `merge_pending_entry` — one buffered column (col_name, col_value via `database_value_dup`, col_version, db_version, site_id, seq) -- `merge_pending_batch` — collects entries for one PK (table, pk, row_exists flag, entries array, statement cache) - -`data->pending_batch` is set to `&batch` (stack-allocated) at the start of `cloudsync_payload_apply`. The INSTEAD OF trigger calls `merge_insert`, which calls `merge_pending_add` instead of `merge_insert_col`. Flush happens at PK/table/db_version boundaries and after the loop. - -### UPDATE vs UPSERT (`row_exists` flag) - -`merge_insert` sets `batch->row_exists = (local_cl != 0)` on the first winning column. At flush time `merge_flush_pending` selects: - -- `row_exists=true` -> `sql_build_update_pk_and_multi_cols` -> `UPDATE docs SET title=? WHERE id=?` -- `row_exists=false` -> `sql_build_upsert_pk_and_multi_cols` -> `INSERT ... ON CONFLICT DO UPDATE` - -Both SQLite and PostgreSQL implement `sql_build_update_pk_and_multi_cols` as a proper UPDATE statement. This is required for SQLiteCloud (which uses the SQLite extension but enforces RLS). - -**Example**: DB A and DB B both have row `id='doc1'` with `user_id='alice'`, `title='Hello'`. Alice updates `title='World'` on A. The payload applied to B contains only `(id, title)`: - -- **UPSERT** (wrong for RLS): `INSERT INTO docs ("id","title") VALUES (?,?) ON CONFLICT DO UPDATE SET "title"=EXCLUDED."title"` — fails INSERT `WITH CHECK` because `user_id` is NULL in the proposed row. -- **UPDATE** (correct): `UPDATE "docs" SET "title"=?2 WHERE "id"=?1` — skips INSERT `WITH CHECK` entirely; the UPDATE `USING` policy checks the existing row which has the correct `user_id`. - -In plain SQLite (no RLS) both produce the same result. The distinction only matters when RLS is enforced (SQLiteCloud, PostgreSQL). - -### Statement cache - -`merge_pending_batch` caches the last prepared statement (`cached_vm`) along with the column combination and `row_exists` flag that produced it. On each flush, `merge_flush_pending` compares the current column names, count, and `row_exists` against the cache: - -- **Cache hit**: `dbvm_reset` + rebind (skip SQL build and `databasevm_prepare`) -- **Cache miss**: finalize old cached statement, build new SQL, prepare, and update cache - -This recovers the precompiled-statement advantage of the old single-column path. In a typical payload where consecutive PKs change the same columns, the cache hit rate is high. - -The cached statement is finalized once at the end of `cloudsync_payload_apply`, not on every flush. - -### `last_payload_db_version` fix - -Moved the update outside the savepoint block so it executes unconditionally: - -```c -if (db_version_changed) { - last_payload_db_version = decoded_context.db_version; -} -``` - -Previously this was inside `if (!in_transaction && db_version_changed)`, which never ran in SPI. - -## Savepoint Architecture - -### Two-level savepoint design - -`cloudsync_payload_apply` uses two layers of savepoints that serve different purposes: - -| Layer | Where | Purpose | -|-------|-------|---------| -| **Outer** (per-db_version) | `cloudsync_payload_apply` loop | Transaction grouping + commit hook trigger (SQLite only) | -| **Inner** (per-PK) | `merge_flush_pending` | RLS error isolation + executor resource cleanup | - -### Outer savepoints: per-db_version in `cloudsync_payload_apply` - -```c -if (!in_savepoint && db_version_changed && !database_in_transaction(data)) { - database_begin_savepoint(data, "cloudsync_payload_apply"); - in_savepoint = true; -} -``` - -These savepoints group rows with the same source `db_version` into one transaction. The `RELEASE` (commit) at each db_version boundary triggers `cloudsync_commit_hook`, which: -- Saves `pending_db_version` as the new `data->db_version` -- Resets `data->seq = 0` - -This ensures unique `(db_version, seq)` tuples in `cloudsync_changes` across groups. - -**In PostgreSQL SPI, these are dead code**: `database_in_transaction()` returns `true` (via `IsTransactionState()`), so the condition `!database_in_transaction(data)` is always false and `in_savepoint` is never set. This is correct because: -1. PostgreSQL has no equivalent commit hook on subtransaction release -2. The SPI transaction from `SPI_connect` already provides transaction context -3. The inner per-PK savepoint handles the RLS isolation PostgreSQL needs - -**Why a single outer savepoint doesn't work**: We tested replacing per-db_version savepoints with a single savepoint wrapping the entire loop. This broke the `(db_version, seq)` uniqueness invariant in SQLite because the commit hook never fired mid-apply — `data->db_version` never advanced and `seq` never reset. - -### Inner savepoints: per-PK in `merge_flush_pending` - -```c -flush_savepoint = (database_begin_savepoint(data, "merge_flush") == DBRES_OK); -// ... database operations ... -cleanup: - if (flush_savepoint) { - if (rc == DBRES_OK) database_commit_savepoint(data, "merge_flush"); - else database_rollback_savepoint(data, "merge_flush"); - } -``` - -Wraps each PK's flush in a savepoint. On failure (e.g. RLS denial), `database_rollback_savepoint` calls `RollbackAndReleaseCurrentSubTransaction()` in PostgreSQL, which properly releases all executor resources (open relations, snapshots, plan cache) acquired during the failed statement. This eliminates the "resource was not closed" warnings that `SPI_finish` previously emitted. - -In SQLite, when the outer per-db_version savepoint is active, these become harmless nested savepoints. - -### Platform behavior summary - -| Environment | Outer savepoint | Inner savepoint | Effect | -|---|---|---|---| -| **PostgreSQL SPI** | Dead code (`in_transaction` always true) | Active — RLS error isolation + resource cleanup | Only inner savepoint runs | -| **SQLite client** | Active — groups writes, triggers commit hook | Active — nested inside outer, harmless | Both run; outer provides transaction grouping | -| **SQLiteCloud** | Active — groups writes, triggers commit hook | Active — RLS error isolation | Both run; each serves its purpose | - -## SPI and Memory Management - -### Nested SPI levels - -`pg_cloudsync_payload_apply` calls `SPI_connect` (level 1). Inside the loop, `databasevm_step` executes `INSERT INTO cloudsync_changes`, which fires the INSTEAD OF trigger. The trigger calls `SPI_connect` (level 2), runs `merge_insert` / `merge_pending_add`, then `SPI_finish` back to level 1. The deferred `merge_flush_pending` runs at level 1. - -### `database_in_transaction()` in SPI - -Always returns true in SPI context (`IsTransactionState()`). This makes the per-db_version savepoints dead code in PostgreSQL and is why `last_payload_db_version` must be updated unconditionally. - -### Error handling in SPI - -When RLS denies a write, PostgreSQL raises an error inside SPI. The inner per-PK savepoint in `merge_flush_pending` catches this: `RollbackAndReleaseCurrentSubTransaction()` properly releases all executor resources. Without the savepoint, `databasevm_step`'s `PG_CATCH` + `FlushErrorState()` would clear the error stack but leave executor resources orphaned, causing `SPI_finish` to emit "resource was not closed" warnings. - -### Batch cleanup paths - -`batch.entries` is heap-allocated via `cloudsync_memory_realloc` and reused across flushes. Each entry's `col_value` (from `database_value_dup`) is freed by `merge_pending_free_entries` on every flush. The entries array, `cached_vm`, and `cached_col_names` are freed once at the end of `cloudsync_payload_apply`. Error paths (`goto cleanup`, early returns) must free all three and call `merge_pending_free_entries` to avoid leaking `col_value` copies. - -## Batch Apply: Pros and Cons - -The batch path is used for all platforms (SQLite client, SQLiteCloud, PostgreSQL), not just when RLS is active. - -**Pros (even without RLS)**: -- Fewer SQL executions: N winning columns per PK become 1 statement instead of N. Each `databasevm_step` involves B-tree lookup, page modification, WAL write. -- Atomicity per PK: all columns for a PK succeed or fail together. - -**Cons**: -- Dynamic SQL per unique column combination (mitigated by the statement cache). -- Memory overhead: `database_value_dup` copies each column value into the buffer. -- Code complexity: batching structs, flush logic, cleanup paths. - -**Why not maintain two paths**: SQLiteCloud uses the SQLite extension with RLS, so the batch path (UPDATE vs UPSERT selection, per-PK savepoints) is required there. Maintaining a separate single-column path for plain SQLite clients would double the code with marginal benefit. - -## Files Changed - -| File | Change | -|------|--------| -| `src/cloudsync.c` | Batch merge structs with statement cache (`cached_vm`, `cached_col_names`), `merge_pending_add`, `merge_flush_pending` (with per-PK savepoint), `merge_pending_free_entries`; `pending_batch` field on context; `row_exists` propagation in `merge_insert`; batch mode in `merge_sentinel_only_insert`; `last_payload_db_version` fix; removed `payload_apply_callback` | -| `src/cloudsync.h` | Removed `CLOUDSYNC_PAYLOAD_APPLY_STEPS` enum | -| `src/database.h` | Added `sql_build_upsert_pk_and_multi_cols`, `sql_build_update_pk_and_multi_cols`; removed callback typedefs | -| `src/sqlite/database_sqlite.c` | Implemented `sql_build_upsert_pk_and_multi_cols` (dynamic SQL); `sql_build_update_pk_and_multi_cols` (delegates to upsert); removed callback functions | -| `src/postgresql/database_postgresql.c` | Implemented `sql_build_update_pk_and_multi_cols` (meta-query against `pg_catalog` generating typed UPDATE) | -| `test/unit.c` | Removed callback code and `do_test_andrea` debug function (fixed 288048-byte memory leak) | -| `test/postgresql/27_rls_batch_merge.sql` | Tests 1-3 (superuser) + Tests 4-6 (authenticated-role RLS enforcement) | -| `docs/postgresql/RLS.md` | Documented INSERT vs UPDATE paths and partial-column RLS interaction | - -## TODO - - - update documentation: RLS.md, README.md and the https://github.com/sqlitecloud/docs repo diff --git a/plans/TODO.md b/plans/TODO.md deleted file mode 100644 index d242187..0000000 --- a/plans/TODO.md +++ /dev/null @@ -1,2 +0,0 @@ -- I need to call cloudsync_update_schema_hash to update the last schema hash when upgrading the library from the 0.8.* version -- Fix cloudsync_begin_alter and cloudsync_commit_alter for PostgreSQL, and we could call them automatically with a trigger on ALTER TABLE \ No newline at end of file diff --git a/src/cloudsync.h b/src/cloudsync.h index ff388e0..7e233c8 100644 --- a/src/cloudsync.h +++ b/src/cloudsync.h @@ -18,7 +18,7 @@ extern "C" { #endif -#define CLOUDSYNC_VERSION "0.9.201" +#define CLOUDSYNC_VERSION "0.9.202" #define CLOUDSYNC_MAX_TABLENAME_LEN 512 #define CLOUDSYNC_VALUE_NOTSET -1 diff --git a/src/postgresql/cloudsync_postgresql.c b/src/postgresql/cloudsync_postgresql.c index 5809454..4860153 100644 --- a/src/postgresql/cloudsync_postgresql.c +++ b/src/postgresql/cloudsync_postgresql.c @@ -73,6 +73,11 @@ static void cloudsync_pg_context_init (cloudsync_context *data) { ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), errmsg("An error occurred while trying to initialize context"))); } + // update schema hash if upgrading from an older version + if (dbutils_settings_check_version(data, NULL) != 0) { + cloudsync_update_schema_hash(data); + } + // make sure to update internal version to current version dbutils_settings_set_key_value(data, CLOUDSYNC_KEY_LIBVERSION, CLOUDSYNC_VERSION); } diff --git a/src/sqlite/cloudsync_sqlite.c b/src/sqlite/cloudsync_sqlite.c index f0b4670..b4fb0fc 100644 --- a/src/sqlite/cloudsync_sqlite.c +++ b/src/sqlite/cloudsync_sqlite.c @@ -1459,6 +1459,11 @@ int dbsync_register_functions (sqlite3 *db, char **pzErrMsg) { return SQLITE_ERROR; } + // update schema hash if upgrading from an older version + if (dbutils_settings_check_version(data, NULL) != 0) { + cloudsync_update_schema_hash(data); + } + // make sure to update internal version to current version dbutils_settings_set_key_value(data, CLOUDSYNC_KEY_LIBVERSION, CLOUDSYNC_VERSION); }