Skip to content

TUR-292: Versioning: count a commit's nodes, edges and dataparts when COMMIT builds them - #1469

Draft
sulaimansuhas wants to merge 1 commit into
suhasnarreddy/tur-298-implement-apoccollsortmultifrom
suhasnarreddy/tur-292-dbhistory-commits-made-by-commit
Draft

sulaimansuhas wants to merge 1 commit into
suhasnarreddy/tur-298-implement-apoccollsortmultifrom
suhasnarreddy/tur-292-dbhistory-commits-made-by-commit

Conversation

@sulaimansuhas

Copy link
Copy Markdown
Contributor

Report the nodes, edges and dataparts a commit added as soon as COMMIT builds it, not only after CHANGE SUBMIT.

Stacked on #1455 (bot stack main): review and merge #1455 first.

@sulaimansuhas

Copy link
Copy Markdown
Contributor Author

Note

AI-generated summary.

Summary

Architecture: COMMIT and CHANGE SUBMIT both reach CommitBuilder::buildAllPending, which now calls the new updateCommitStats to write the node, edge and datapart counts that db.history and db.describeCommit read

db.history reads nodeCount, edgeCount and partCount from the Commit. Only CommitBuilder::build() wrote them, and only CHANGE SUBMIT calls build(). COMMIT calls buildAllPending() alone. So inside an open change, a committed commit reported 0 / 0 / 0 until the submit.

The count loop moves out of build() into a new private updateCommitStats(), called at the end of buildAllPending(). Both paths run it:

 Change::commit                      (COMMIT)
   flushWriteBuffer
   buildAllPending
     load each DataPartBuilder
+    updateCommitStats               sum node/edge container sizes over commitDataparts

 VersionController submit            (CHANGE SUBMIT)
   rebase, if main moved
   build
     buildAllPending
+      updateCommitStats             recounts: same values, plus any parts flushed at submit
     journal().finalise()
-    count loop over commitDataparts

The loop body is the same as before, except that it iterates the DataPartSpan directly and reads it from _commitData rather than through _commit->data().

Data flow: COMMIT builds the pending dataparts, updateCommitStats writes 2 / 1 / 1 to the Commit, and db.history reads 2 / 1 / 1 before SUBMIT where it read 0 / 0 / 0

The test fixture WriteQueryTest gets a runQuery(query, changeID, sink) overload, so a test can read rows inside a change. runWrite now calls it and does not repeat the execute call.

Evidence

New target test_query_ir_change_commit_history (test/query/ir/write/ChangeCommitHistoryTest.cpp, 9 cases). Each case reads CALL db.history() YIELD nodeCount, edgeCount, partCount head-first. The first row is the open tip, which is always 0 / 0 / 0.

  • Before: I did not run the new test on the base commit. From the code, every committed row read inside the change is 0 / 0 / 0, because only build() wrote the counts:
    CREATE (a:Person)-[:KNOWS]->(b:Person); COMMIT; CALL db.history() in the change
      -> [0,0,0] [0,0,0] <base history...>
    
    After: I ran the binary built from this commit: 9 of 9 pass.
    CREATE (a:Person)-[:KNOWS]->(b:Person); COMMIT; CALL db.history() in the change
      -> [0,0,0] [2,1,1] <base history...>
    
    [ OK ] committedCreateReportsItsNodesEdgesAndPart                 [2,1,1]
    [ OK ] committedCreateBetweenExistingNodesCountsOnlyWhatItAdded   [1,2,1]  1 new node, 2 edges to existing nodes
    [ OK ] everyCommitOfAChangeReportsItsOwnCounts                    [3,0,1] then [2,1,1]
    [ OK ] oneCommitOfSeveralQueriesReportsTheirSum                   [3,1,1]
    [ OK ] submitKeepsTheCountsTheChangeReported                      change rows == head rows after SUBMIT
    [ OK ] rebaseOnSubmitKeepsTheCounts                               main moves first; still [2,1,1] after rebase
    [ OK ] commitWithCreatesAndUpdatesCountsBothParts                 CREATE + SET -> [1,0,2]
    [ OK ] describeCommitReportsACommittedCommit                      db.describeCommit -> [2,1,1]
    [ OK ] openTipReportsNothingBeforeCommit                          staged writes, no COMMIT -> tip [0,0,0]
    [  PASSED  ] 9 tests.
    

I did not run the rest of ctest.

Merge Danger

Door: two-way

No format change. The counts were already in the commit metadata that CommitMetaDataDumper writes. A commit on main has the same values as before, because build() still runs buildAllPending(), which now does the count. A revert restores the old timing.

Blast Radius: small

Only readers of Commit::getNumNodes/getNumEdges/getNumDataParts on a commit that is still in an open change see a difference: db.history, db.describeCommit and GraphComparator. They now see real counts where they saw zeros. The count runs once more per COMMIT. Its cost is one loop over the commit's own dataparts.

This branch has not been deployed

No deployments
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