Skip to content

payload_chunks: max_window_bytes does not bound scan work over RLS-hidden rows #81

Description

@andinux

Summary

max_window_bytes (#77) bounds the bytes a cloudsync_payload_chunks() window emits. It does not bound the work spent scanning rows the caller cannot see. Rows hidden by row-level security are read and checked against the rule, then dropped, and they spend no budget. So for a user who sees a small part of a large history, a window can scan the whole history without ever reaching the cap.

The duration of a CloudSync prepare job then depends on the scan, not the budget. With an expensive RLS rule the job can exceed the server's job timeout (1800 s on CloudSync). A timed-out job keeps nothing, and the next attempt restarts from zero: the same loop the budget was built to prevent, reached through RLS instead of data volume.

This is a follow-up, not a blocker for 1.2.0: the cap does what it was designed to do.

Evidence (staging, SQLite Cloud, extension 1.2.0 from #77, CloudSync sqliteai/cloudsync#63)

Test table capped_sync_test: 1,121 rows (3,363 change rows, 3 synced columns), about 124 MB of incompressible data, 164 db_versions. Each run was a fresh device syncing from db_version 0 with a user access token (user scope), with the budget set to 1 GiB, which is never reached here.

RLS SELECT rule Visible Emitted Change rows scanned Prepare job duration_ms
none all 124.3 MB, 25 chunks 3,363 11,105
A: shape = 'tail' 50 rows 5.0 MB, 1 chunk 3,363 (3,213 hidden) 1,556
B: EXISTS (SELECT 1 FROM rls_visible WHERE visible_shape = shape), 10,000-row table without an index 50 rows 5.0 MB, 1 chunk 3,363 (3,213 hidden) 6,789
B, nothing new (since = 164) n/a nothing n/a 379

What it shows:

  • Cost per hidden change row: about 0.2 ms with a simple column rule, and about 1.5–2 ms with a subquery rule. The emitted output is identical in both cases.
  • Extrapolation: at the rule-B rate, a window with about 1M hidden change rows takes around 30 minutes, past the timeout, whatever max_window_bytes is set to.
  • The whole-history MAX(db_version) query was not affected by the rule on SQLite Cloud (379 ms, the same as without RLS). It presumably doesn't read row values, so the rule isn't checked. PostgreSQL is untested: there, cloudsync_changes_select joins the base table for every change row (build_union_sql), so the rule applies to every row scanned, and the MAX query covers the whole history from version 0.

Why this isn't a small change

The cap can only end a window at a db_version after it has emitted something. payload_chunks_apply_window_cap runs after a chunk is built, and the #77 docs state that a window never ends empty, which is what guarantees progress. Bounding scan work means stopping when the scan has done enough work even if nothing visible was found, which means a window can end empty but still advance the watermark.

That needs a way to say "nothing for you up to watermark W" that every layer understands:

  • The extension would have to emit a final row with no payload rows, is_final = 1, a lowered watermark_db_version, and window_capped = 1, or an equivalent.
  • The CloudSync server currently treats a window with no chunks as "no changes" and does not advance the device's checkpoint. It would have to store and serve an empty capped chunk.
  • The client would have to accept a final chunk whose payload has zero rows and advance its receive checkpoint to that watermark. Whether cloudsync_payload_apply accepts, and checkpoints on, an empty payload today needs checking before choosing a design.

Options

  1. Count scanned rows toward the budget, but still end windows only after something was emitted. Hidden rows would spend budget at a nominal per-row cost, or through a separate max_window_rows. This is the smallest change and needs no protocol work, and it covers users who see some of the history regularly. It does not cover a long stretch where the user sees nothing: the window still can't end until a visible row appears.
  2. Allow empty capped windows, as described above. This covers every case, including users who see nothing. It needs coordinated changes in the extension, the server and the client, and the zero-row payload question answered first.
  3. A time budget (max_window_ms). It bounds everything, but windows then depend on load and aren't reproducible, and it has the same empty-window problem as option 2. Probably not worth it.

My leaning: option 1 soon, because it's cheap and covers the common case, then option 2 if production shows windows that are mostly hidden.

Mitigations available now

  • Document for customers: columns used inside RLS subqueries should be indexed. Rule B's cost comes from scanning 10,000 unindexed rows for every change row; with an index it should be close to rule A. (Not yet confirmed on staging.)
  • Monitor on the CloudSync side: check_chunks_prepared (with window_bytes) together with job_completed.duration_ms. A long job with a small window_bytes is this case.

Also worth measuring

  • The PostgreSQL cost of the same rules. That needs a PostgreSQL tenant; staging has none today.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions