Skip to content

fix: publish grid cache cells atomically and preserve invalidations - #9

Draft
afonsobspinto wants to merge 1 commit into
metacellfrom
feature/grid-cache-transactions
Draft

afonsobspinto wants to merge 1 commit into
metacellfrom
feature/grid-cache-transactions

Conversation

@afonsobspinto

Copy link
Copy Markdown
Member

Purpose

Draft for internal review before opening against upstream catmaid/CATMAID:dev. The shared helper and cache-worker fixes are already included in feature/new-ingestion (PR #7). This PR isolates those runtime changes and adds PostgreSQL regression tests for review before submitting upstream.

What changed and why

Adds catmaid.control.grid_cache.locked_grid_cell() and uses it in the normal GridWorker:

  1. Acquire a transaction advisory lock for the grid ID and cell coordinates before reading source data. Cooperating refreshes of the same cell serialize; different cells can still refresh concurrently.
  2. Generate and publish all configured payload formats in one transaction.
  3. Remember the dirty row's ID and PostgreSQL row version (xmin) before generation. Clear only that version after successful publication, preserving newer invalidations.
  4. Record the generation start using clock_timestamp() after acquiring the lock.
  5. Send the worker's existing client notifications after the cell transaction commits.

The helper is shared with the ingestion rebuild in feature/new-ingestion; this internal PR contains no importer or provenance code. It does not change worker node limits, grid settings, payload selection or query geometry.

Before and after examples

1. An edit arrives during generation

A node moves from X=100 to X=200 while its cell is refreshing.

Step Before After
Worker reads source data Reads X=100. Reads X=100 and remembers the dirty row's version.
User moves the node The source becomes X=200 and the dirty row changes. Same.
Worker publishes Its result can still contain X=100. Same temporary result is possible.
Worker acknowledges completion Deletes the dirty marker unconditionally, potentially making stale data appear clean. Leaves the changed dirty marker intact so the cell remains marked for refresh.

Existing notifications may trigger another refresh in either version; the fix prevents losing the database's evidence that another refresh is needed. Comparing xmin also works when two invalidations have identical timestamps or a longer-running transaction records an older timestamp.

2. A slower refresh overwrites a newer result

Before:

Worker A reads older source data and pauses.
Worker B reads newer data and publishes it.
Worker A resumes and overwrites B's payload with its older result.

After:

Worker A locks the cell, reads, publishes, and commits.
Worker B then acquires the lock, reads current data, and publishes.

This also coordinates a normal worker with an importer using the same helper. Making a Neuroglass layer inactive does not stop already queued cache jobs.

3. MessagePack serialization fails after JSON was written

Before After
JSON can commit before MessagePack fails, leaving different payload formats from different generations. The cell transaction rolls back all payload writes and preserves the dirty marker.
Readers can observe a partially updated set of formats. Readers see the previous committed payloads until the whole cell commits.

For a grid storing only MessagePack, the multiple-format benefit is smaller; same-cell serialization and safe invalidation acknowledgement still apply.

4. Generation waits for a lock

If a transaction starts at 10:00 but acquires its cell lock at 10:02, PostgreSQL's now() still reports 10:00. The helper records generation beginning at 10:02, after the wait. This supports accurate freshness checks with the locking protocol.

Scope and limitations

Advisory locking coordinates callers using this helper; other cache-writing paths do not automatically participate. It does not lock skeleton editing or ordinary reads. An offline full rebuild can still leave missing or partial coverage, so the target ingestion layer must remain inactive through final verification.

The rectangular-cell bounds correction is reviewed separately in PR #8.

Validation

Adds nine PostgreSQL transaction/concurrency tests using independent database connections and explicit synchronization. They cover:

  • Same-cell serialization before source reads and generation timestamps after lock waits.
  • Concurrent progress on different cells.
  • Updated dirty rows with unchanged timestamps, newly inserted invalidations, and invalidations committing while acknowledgement waits on a row lock.
  • Visibility of all old formats until commit, rollback, and retry.
  • The actual worker's lock/transaction integration, notifications after commit, and rollback when MessagePack serialization fails after earlier format writes.

All nine pass on PostgreSQL 16/PostGIS with the internal metacell runtime (Django 4.2.30/psycopg2) and were also validated on upstream dev (Django 5.2.17/psycopg 3.3.6).

Negative controls confirmed that the two worker regressions fail with the original upstream worker, and two invalidation regressions fail when dirty-row version checks are removed.

python manage.py test catmaid.tests.test_grid_cache_transactions

No database migrations. This internal branch contains one fix commit directly on metacell; cherry-pick it onto current upstream dev after internal review.

Coordinate cache workers and other cooperating refresh callers with one advisory lock and transaction per cell. Acknowledge only the dirty-row version observed before generation, preserve newer invalidations, and record generation time after acquiring the lock.

Add nine PostgreSQL transaction and concurrency tests covering source-read serialization, independent cells, invalidation races, atomic payload visibility, rollback, and worker integration. Validated on PostgreSQL 16/PostGIS with Django 5.2.17 and psycopg 3.3.6; negative controls reproduce failures with the upstream worker and unsafe dirty-marker removal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant