HarperFast / HarperFast/rocksdb-js
writeBatch's open-retry loop is unbounded, so a persistently failing volume spins through sequence numbers
- Dominant language
- C++
- Stars
- 21
- Forks
- 2
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 36
Description
Noticed while fixing #748; pre-existing, not introduced by that change.
`TransactionLogStore::writeBatch` retries in two nested loops with no bound:
```
while (!batch.isComplete()) {
while (logFile == nullptr) {
logFile = this->getLogFile(currentSequenceNumber);
try { if (!logFile->isOpen()) logFile->open(latestTimestamp); }
catch { writeFailures++; logFile = nullptr; }
if (logFile == nullptr || maxFileSize > 0) rotateToNextSequence(logFile);
}
...
}
```
Every open failure advances the sequence number and tries again. Under a condition that fails *every* open — a full volume, an exhausted quota, a directory that lost write permission — there is nothing to stop it: each iteration attempts to create another log file, fails, and burns a sequence number. The commit never returns an error to the caller, it just spins.
The same loop is the fallback for a file that reports no write progress, so #748's new "retire this file and rotate" path reaches it too, as does a short header write. Those are additional routes into the loop rather than new failure modes — `openFile()` already threw under ENOSPC before any of that work.
Worth deciding: a retry bound (or a bound on consecutive open failures) after which `writeBatch` throws, so a full disk surfaces as a rejected commit instead of a spin. The tricky part is picking a bound that cannot reject a commit that a legitimate rotation would have satisfied — a batch genuinely can span several files.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor guide
Research direction
Start with TransactionLogStore::writeBatch and trace getLogFile, openFile(), and rotateToNextSequence, including the no-write-progress and short-header-write paths. Reproduce persistent open failure conditions such as ENOSPC, then determine a retry bound that still permits a batch to span several files. Done means writeBatch returns an error instead of spinning while legitimate rotation remains possible.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100