refactor: skip no-op edge CSR compaction after COPY

[zhanglei1949]

Related Issues

Fixes #1069

What does this PR do?

Skips redundant edge CSR compaction for plain timestamp-zero COPY targets while preserving compaction for sort-key tables and edge tables carrying ordinary MVCC writes or deletes.

What changes in this PR?

• Track the durable CSR normalization state in each EdgeTable as needs_csr_normalization_.
• Mark the state from ordinary add, update, and delete entry points, including EdgeTableView::AddEdge used by MVCC WAL ingestion; timestamp-zero batch COPY remains normalized.
• Persist and clone the state across COW and incremental checkpoint reopen.
• For legacy manifests without the scalar, treat base timestamp zero as clean and nonzero as requiring normalization.
• Keep sort-key edge tables on the compaction and sort path.
• Consult EdgeTable::NeedsCompaction() from both bulk-COPY finalization and PropertyGraph::Compact(), skipping EdgeTable::Compact() only when the target has no sort key and its CSR state is normalized.

Why is an EdgeTable-local normalization state necessary?

DirtyTracker and CSR normalization answer different questions and have independent lifecycles:

• DirtyTracker controls which table modules must be written by a checkpoint and is cleared after an incremental checkpoint.
• needs_csr_normalization_ records whether CSR timestamps or tombstones still require a later O(E) normalization scan; that requirement can survive the checkpoint that clears persistence dirtiness.
• A timestamp-zero COPY target is dirty for persistence but already normalized, while a table reopened after ordinary MVCC writes can be clean for persistence but still require normalization.

Keeping the state in EdgeTable lets the object that owns both CSR directions maintain the invariant at every mutation entry point and persist it across reopen. EdgeTableView shares that state so view-based WAL ingestion cannot bypass the invariant. Inferring the same fact from CSR contents would require scanning adjacency data and would defeat the optimization.

Correctness coverage

• Verifies direct EdgeTable mutations and GraphView::AddEdge mark the normalization state, while timestamp-zero view insertion keeps it clear and Compact() resets it.
• Verifies legacy manifest fallback for base timestamps zero and nonzero.
• Verifies the state survives incremental checkpoint reopen.
• Uses counting CSRs to prove clean plain targets execute zero compact calls and dirty targets compact both directions.
• Covers both bulk-COPY finalization and the full PropertyGraph::Compact() path.
• Runs a real direct CSV COPY for a plain edge table and verifies data before and after checkpoint reopen without Session.

Testing

• EdgeTableTest.*: 20 tests passed.
• GraphViewTest.*: 14 tests passed.
• APIndexTest.*: 30 tests passed.
• Focused Python COPY persistence tests: 2 tests passed.
• clang-format --dry-run --Werror on the changed C++ files: passed.
• git diff --check: passed.
• Coverage report was not generated because the current build has ENABLE_GCOV=OFF and fastcov/lcov are unavailable.

Notes

The optimization is intentionally conservative: sort-key tables always compact, and legacy nonzero checkpoints missing the normalization scalar may perform one extra compaction rather than risk skipping required MVCC normalization.

[Copilot]

🟡 Changes recommended

Critical correctness issues remain in MVCC edge marking and unbundled updates, plus a moderate stale-state issue.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR optimizes checkpoint finalization by skipping redundant CSR compaction for clean, plain edge tables while preserving required compaction paths.

Changes:

• Tracks and persists edge-table compaction state.
• Adds conservative legacy-manifest fallback behavior.
• Adds C++ and Python persistence and compaction tests.

File summaries

File	Summary
tools/python_bind/tests/test_persistence.py	Tests direct CSV COPY persistence across checkpoint reopen.
tests/storage/test_edge_table.cc	Tests mutation tracking and legacy fallback behavior.
tests/storage/test_ap_index.cc	Tests persistence and compaction call counts.
src/transaction/cow_graph_workspace.cc	Skips compaction for clean plain-edge targets; critical issue remains for MVCC insert/WAL edges bypassing the marker.
src/storages/graph/edge_table.cc	Tracks and persists compaction state; critical issue remains because unbundled property updates unnecessarily mark tables dirty.
src/main/checkpoint_coordinator.cc	Documents checkpoint finalization behavior.
include/neug/storages/graph/edge_table.h	Defines compaction-state tracking; moderate issue remains because CSR rebuild paths do not clear the state.

Review details

Suppressed comments (2)

include/neug/storages/graph/edge_table.h:220

• The new bit is not cleared by the existing CSR-rebuild paths dropAndCreateNewBundledCSR and dropAndCreateNewUnbundledCSR: both export only live edges and reinsert them with timestamp 0, but leave needs_csr_compaction_ unchanged. After an ordinary MVCC mutation followed by edge-property DDL and then COPY, this keeps a now-normalized plain table dirty and causes another redundant compaction, defeating the optimization. Clear the state after these rebuilds (or centralize the reset).

  // Batch COPY appends checkpoint-normalized CSR entries at timestamp zero.
  // Ordinary writes and deletes require a later CSR scan to normalize
  // timestamps or remove tombstones. Persist this bit across incremental
  // checkpoint reopen so a later bulk load cannot skip required compaction.
  std::atomic<bool> needs_csr_compaction_{false};

src/storages/graph/edge_table.cc:837

• This flag is not updated for every ordinary edge insertion: GraphView::EdgeTableView::AddEdge writes through insert_edge_into_csr_internal directly (src/storages/graph/graph_view.cc:200-205), and MvccInsertTransaction::IngestWal uses that path (src/transaction/mvcc_insert_transaction.cc:239-244). After such an MVCC insert, a later COPY on the same table can see needs_csr_compaction_ == false and skip the required compaction. Route the view insertion through the state-tracking path or propagate the flag into EdgeTableView.

  if (ts != 0) {
    needs_csr_compaction_.store(true);
  }

• Files reviewed: 7/7 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.