Create draft plans for addressing each limitation
This commit is contained in:
1 parent
5784de249d
commit
5f879fd9a4
8 files changed
+954
-10
No files matched your search
@@ -0,0 +1,107 @@
|
||||
# Write statistics
|
||||
|
||||
## Problem
|
||||
|
||||
Neo4j reports what a write query changed in the `stats` entry of the final `PULL`
|
||||
summary (`nodes-created`, `relationships-deleted`, `properties-set`, ...).
|
||||
neo4j-browser shows these as "Created 2 nodes, set 4 properties". boltage sends no
|
||||
`stats` (`BoltConnection.pull`, `boltage/bolt.py:327`), so the browser says
|
||||
"(no changes, no records)". Drivers' `summary.counters.contains_updates` is always
|
||||
false, which also breaks code that checks it.
|
||||
|
||||
AGE's `cypher()` does not report counts, and AGE writes label tables without firing
|
||||
triggers, so triggers cannot count either.
|
||||
|
||||
## Approach: PostgreSQL's transaction-local table statistics
|
||||
|
||||
`pg_stat_xact_user_tables` has, per table, the number of rows inserted, updated
|
||||
and deleted **by the current transaction so far** (`n_tup_ins`, `n_tup_upd`,
|
||||
`n_tup_del`). Every AGE label is a table in the graph's schema, and
|
||||
`ag_catalog.ag_label` says whether it holds vertices or edges. Snapshot those
|
||||
counters for the graph's schema before and after the query and take the difference.
|
||||
|
||||
1. **Spike first.** Confirm on the supported PostgreSQL versions (the image uses 18):
|
||||
- the counters include the changes of a statement that has just finished, within
|
||||
the same transaction;
|
||||
- a non-superuser can read them for tables they wrote to;
|
||||
- how AGE's `SET` and `REMOVE` show up (an `UPDATE` of the row, so `n_tup_upd`),
|
||||
and `DETACH DELETE` (deletes on edge tables and the vertex table);
|
||||
- `MERGE` that matches counts nothing, `MERGE` that creates counts inserts.
|
||||
2. **Transaction per auto-commit query.** The counters reset at commit, and the
|
||||
connection runs in autocommit mode (`boltage/backend.py:126`). Wrap each
|
||||
auto-commit Cypher query in `BEGIN` / `COMMIT` in `Session.run`
|
||||
(see [shared groundwork](README.md#shared-groundwork)). Inside an explicit Bolt
|
||||
transaction nothing changes: the counters accumulate, and the diff per query
|
||||
still works.
|
||||
3. **Snapshot query.** One query, run before and after:
|
||||
|
||||
```sql
|
||||
SELECT l.kind, s.relname <> ALL(%(defaults)s) AS labelled,
|
||||
sum(s.n_tup_ins), sum(s.n_tup_upd), sum(s.n_tup_del)
|
||||
FROM pg_stat_xact_user_tables s
|
||||
JOIN ag_catalog.ag_label l ON l.relation = s.relid
|
||||
JOIN ag_catalog.ag_graph g ON g.graphid = l.graph
|
||||
WHERE g.name = %(graph)s
|
||||
GROUP BY 1, 2
|
||||
```
|
||||
|
||||
To avoid two extra round trips per query, send the "before" snapshot in the same
|
||||
pipeline as `BEGIN` (psycopg's `conn.pipeline()`), and skip both snapshots for
|
||||
queries that cannot write (no `CREATE`/`MERGE`/`SET`/`DELETE`/`REMOVE` keyword).
|
||||
The existing keyword check at `boltage/backend.py:232` runs on the raw text, so a
|
||||
string literal like `'SET'` counts as a write; run it on `scan().masked` instead.
|
||||
A false positive only costs the snapshots, a false negative loses the counts.
|
||||
4. **Mapping to Neo4j counters:**
|
||||
|
||||
| Neo4j counter | Source |
|
||||
| --- | --- |
|
||||
| `nodes-created` / `nodes-deleted` | inserts / deletes on vertex tables |
|
||||
| `relationships-created` / `relationships-deleted` | inserts / deletes on edge tables |
|
||||
| `labels-added` | inserts on vertex tables other than `_ag_label_vertex` (AGE has one label per node) |
|
||||
| `labels-removed` | deletes on vertex tables other than `_ag_label_vertex` |
|
||||
| `properties-set` | see below |
|
||||
| `contains-updates` | true when any of the above is non-zero |
|
||||
|
||||
Only send non-zero counters, like Neo4j, plus `contains-updates`.
|
||||
5. **`properties-set`** has no exact source: an update rewrites a row's whole
|
||||
`properties` map. Start with an approximation and document it:
|
||||
- for inserts: not counted (Neo4j counts the properties in `CREATE`; leaving this
|
||||
out under-reports, but never claims changes that did not happen);
|
||||
- for updates: the number of updated rows.
|
||||
A more exact count would need comparing old and new `properties` maps, which AGE
|
||||
does not expose. Revisit if users ask for it.
|
||||
6. **System commands.** `CREATE DATABASE` / `DROP DATABASE` (and later user
|
||||
management, plan 04) report `system-updates: 1` and `contains-system-updates: true`.
|
||||
7. **Plumbing.** Add a `stats` dict to `Result`, filled in `Session._run_cypher` and
|
||||
sent by `BoltConnection.pull` in the final `SUCCESS`. Use the counts to set
|
||||
`query_type` (`r`, `w` or `rw`, depending on whether there are columns and writes)
|
||||
instead of the regex.
|
||||
|
||||
With streaming (plan 06) the "after" snapshot must be taken once the cursor is
|
||||
exhausted or drained, just before the final summary.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
- **Counting rows before and after** with `count(*)` per label table: slow on large
|
||||
graphs, wrong under concurrent writers, and blind to updates.
|
||||
- **Rewriting the query to count**, for example appending a `RETURN count(...)`:
|
||||
changes the query's result and does not work for queries that already return.
|
||||
- **Statement-level triggers with transition tables:** AGE does not fire triggers on
|
||||
its label tables.
|
||||
|
||||
## Tests
|
||||
|
||||
Smoke test, using the driver's `summary.counters`:
|
||||
|
||||
- `CREATE (a:Person)-[:KNOWS]->(b:Person)`: 2 nodes, 1 relationship, 2 labels,
|
||||
`contains_updates`.
|
||||
- `MATCH (n:Person) SET n.x = 1`: properties set equals the number of persons.
|
||||
- `MATCH (n:Person) DETACH DELETE n`: nodes and relationships deleted.
|
||||
- `MATCH (n) RETURN n`: no updates.
|
||||
- Same queries in an explicit transaction, two queries in one transaction report
|
||||
their own counts.
|
||||
- `CREATE DATABASE`: system updates.
|
||||
|
||||
## README changes
|
||||
|
||||
Remove the write statistics bullet; add a note that `properties-set` is approximate.
|
||||
Reference in new issue
Block a user