Skip to content

Latest commit

 

History

History
122 lines (86 loc) · 5.41 KB

README.md

File metadata and controls

122 lines (86 loc) · 5.41 KB

Case Study: Benchmark Failure Analysis - Issue #3

Executive Summary

The Neo4j vs Doublets benchmark suite was failing during the delete benchmark with error NotExists(4000). Root cause analysis revealed that the Transaction wrapper implementation was a non-functional stub that always returned errors for delete operations, causing the benchmark to panic.

Timeline of Events

Time Event
2025-12-22 12:22:51 First failed run detected (run-20431789056)
2025-12-22 13:20:15 Second failed run (run-20433102564)
2025-12-22 16:06:33 Third failed run referenced in issue (run-20437258944)

All three failures exhibited the same error pattern:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: NotExists(4000)', benches/benchmarks/delete.rs:23:9

Technical Analysis

Error Location

The panic occurred at benches/benchmarks/delete.rs:23, which corresponds to the bench! macro invocation. The actual error was propagated from inside the macro where fork.delete(id)? was called.

Root Cause

The root cause was in rust/src/transaction.rs. The Transaction struct was intended to be a wrapper around the Client to provide transactional Neo4j operations. However, the implementation was a non-functional stub:

Original problematic code:

// transaction.rs lines 55-74
fn create_links(&mut self, _query: &[T], handler: WriteHandler<T>) -> std::result::Result<Flow, Error<T>> {
    // Does nothing - just returns empty handler
    Ok(handler(Link::nothing(), Link::nothing()))
}

fn delete_links(&mut self, query: &[T], _handler: WriteHandler<T>) -> std::result::Result<Flow, Error<T>> {
    // Always returns NotExists error!
    Err(Error::NotExists(query[0]))
}

Execution Flow

  1. Create benchmark runs first for both Neo4j_NonTransaction and Neo4j_Transaction

    • Neo4j_NonTransaction: Uses Client which properly creates links in Neo4j
    • Neo4j_Transaction: Uses Transaction which silently does nothing (returns empty handler)
    • Create benchmark "passes" for both because no error is returned

  2. Delete benchmark runs second

    • Neo4j_NonTransaction: Completes successfully (links exist, deletes work)
    • Neo4j_Transaction: Fails immediately because:
      • create_links didn't actually create any links
      • delete_links always returns Err(Error::NotExists(id))

  3. The tri! macro wraps the benchmark and calls .unwrap() on the result, causing the panic

Why Error was NotExists(4000)?

The delete benchmark loops from ID 4000 down to 3000 in reverse order:

for id in (BACKGROUND_LINKS..=BACKGROUND_LINKS + 1_000).rev() {
    let _ = elapsed! {fork.delete(id)?};
}

Where BACKGROUND_LINKS = 3000. So the first attempted delete is ID 4000, which immediately returns Err(NotExists(4000)).

Solution Implemented

The fix properly implements Transaction to delegate all operations to the underlying Client:

  1. Made Client API public: Added public accessor methods and made response types public

    • pub fn host(&self) -> &str
    • pub fn port(&self) -> u16
    • pub fn auth(&self) -> &str
    • pub fn constants(&self) -> &LinksConstants<T>
    • pub fn fetch_next_id(&self) -> i64
    • pub fn execute_cypher(&self, ...) -> Result<CypherResponse>
    • Made CypherResponse, QueryResult, RowData, CypherError public

  2. Rewrote Transaction: Full delegation to Client for all Links/Doublets operations

    • create_links: Now properly creates links via Client's execute_cypher
    • delete_links: Now properly queries and deletes via Client
    • All other operations also properly delegated

Design Note

As the comment in transaction.rs explains:

In the HTTP-based approach using /db/neo4j/tx/commit endpoint, all requests are auto-committed transactions. This wrapper exists for API compatibility to benchmark "transactional" Neo4j operations.

The Transaction and NonTransaction benchmarks will now produce similar results since the HTTP API auto-commits. For true transactional semantics, multi-request transaction endpoints would need to be used (see Neo4j HTTP Transaction API).

Files Changed

File Changes
rust/src/client.rs Made CypherResponse/QueryResult/RowData/CypherError public, added accessor methods
rust/src/transaction.rs Complete rewrite to delegate to Client

Lessons Learned

  1. Stub implementations should fail explicitly: The original stub silently returned success for creates but explicit failure for deletes, causing confusing behavior.

  2. Integration tests needed: Unit tests for individual components would not have caught this since the stub "worked" in isolation.

  3. CI logs are essential: The CI logs clearly showed the exact error and location, enabling quick diagnosis.

References

CI Logs Archive

The following CI run logs have been archived in this case study:

  • logs/run-20431789056.log - First failure
  • logs/run-20433102564.log - Second failure
  • logs/run-20437258944.log - Third failure (referenced in issue)