Skip to content

[BUG] edge_num_ double-subtraction in batch_delete_edges due to missing already-deleted guard #720

Description

@zhanglei1949

Describe the bug

edge_num_ can be incorrectly decremented multiple times for already-deleted edges, causing the counter to underflow (wrap around for uint64_t), leading to incorrect edge count reporting and potential downstream failures.

Affected locations

1. SingleMutableCsr::batch_delete_edges (offset version) — src/storages/csr/mutable_csr.cc:676-697

for (const auto& edge : edge_list) {
    vid_t src = edge.first;
    if (src >= vnum) { continue; }
    auto& nbr = data[src];
    assert(edge.second == 0);
    nbr.timestamp.store(std::numeric_limits<timestamp_t>::max());
    edge_num_.fetch_sub(1, std::memory_order_relaxed);  // ← unconditional!
}

edge_num_.fetch_sub(1) is called without checking whether the edge was already deleted (timestamp already at max). If the same edge is deleted twice (e.g., through retry logic or duplicate entries), the counter is decremented twice.

2. MutableCsr::batch_delete_edges (src/dst version) — src/storages/csr/mutable_csr.cc:363-370

if (pair.second.find(write_ptr->neighbor) != pair.second.end()) {
    write_ptr->timestamp.store(std::numeric_limits<timestamp_t>::max());
    edge_num_.fetch_sub(1, std::memory_order_relaxed);  // ← unconditional!
}

Same issue — no check for already-deleted state before decrementing.

Contrast with correct implementations

  • MutableCsr::batch_delete_edges (offset version, line 399-405) correctly checks before decrementing.
  • SingleMutableCsr::batch_delete_edges (src/dst version, line 668-672) correctly checks before decrementing.

Only the two locations above are missing the guard.

Expected behavior

All batch_delete_edges variants should check timestamp != max before calling edge_num_.fetch_sub(1), consistent with the other implementations in the same file.

Suggested fix

For SingleMutableCsr::batch_delete_edges (offset version):

if (nbr.timestamp.load() != std::numeric_limits<timestamp_t>::max()) {
    edge_num_.fetch_sub(1, std::memory_order_relaxed);
}
nbr.timestamp.store(std::numeric_limits<timestamp_t>::max());

For MutableCsr::batch_delete_edges (src/dst version):

if (write_ptr->timestamp.load() != std::numeric_limits<timestamp_t>::max()) {
    edge_num_.fetch_sub(1, std::memory_order_relaxed);
}
write_ptr->timestamp.store(std::numeric_limits<timestamp_t>::max());

Impact

  • Severity: High — edge count corruption can trigger false THROW_STORAGE_EXCEPTION in compact() (line 204-211) and Open() (line 79-87), causing storage layer failures.
  • Likelihood: Medium — requires duplicate edge deletion, which can occur during transaction retries or WAL replay.

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingstoreStorage layer

Type

Projects

Status
To do

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions