Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified Docs/bench_rust.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified Docs/bench_rust_log_scale.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ The results below represent the amount of time (ns) the operation takes per iter

| Operation | Doublets United Volatile | Doublets United NonVolatile | Doublets Split Volatile | Doublets Split NonVolatile | Neo4j NonTransaction | Neo4j Transaction |
|---------------|--------------------------|-----------------------------|-------------------------|----------------------------|----------------------|-------------------|
| Create | 97789 (0.3x faster) | 100438 (0.3x faster) | 86465 (0.4x faster) | 83558 (0.4x faster) | 3107850791 | 31740 |
| Create | 96754 (31302.3x faster) | 99047 (30577.7x faster) | 84563 (35815.0x faster) | 84223 (35959.6x faster) | 3112975781 | 3028627101 |
| Update | N/A | N/A | N/A | N/A | N/A | N/A |
| Delete | N/A | N/A | N/A | N/A | 1868497988 | N/A |
| Delete | N/A | N/A | N/A | N/A | 1913172623 | N/A |
| Each All | N/A | N/A | N/A | N/A | N/A | N/A |
| Each Identity | N/A | N/A | N/A | N/A | N/A | N/A |
| Each Concrete | N/A | N/A | N/A | N/A | N/A | N/A |
Expand Down
122 changes: 122 additions & 0 deletions docs/case-studies/issue-3/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
# 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:**
```rust
// 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:
```rust
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](https://neo4j.com/docs/http-api/current/transactions/)).

## 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

- [Issue #3](https://github.com/linksplatform/Comparisons.Neo4jVSDoublets/issues/3)
- [PR #4](https://github.com/linksplatform/Comparisons.Neo4jVSDoublets/pull/4)
- [Neo4j HTTP Transaction API](https://neo4j.com/docs/http-api/current/transactions/)
- [Neo4j Cypher Transaction API](https://neo4j.com/docs/http-api/current/actions/)

## 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)
Loading