Repository navigation
FileStore: close the active segment before threshold compaction - #11
Closed
genemichael wants to merge 1 commit into
Closed
genemichael wants to merge 1 commit into
genemichael wants to merge 1 commit into
Conversation
compact_if_threshold() called compact() with active_file still open. rotate_segment_if_needed() closes it first, but the threshold path did not, so on ESP-IDF LittleFS every threshold-triggered compaction failed to delete the active segment: E esp_littlefs: Failed to unlink path "./hashlist_store/seg1.dat". Has open FD. Plain POSIX allows unlinking an open file, which is why this never showed up off-device. Flush and close active_file before compact(), and reopen the current segment if compaction fails or is skipped so appends continue. Seen on microReticulum_Firmware (Heltec V4) relaying link traffic, where the 100-record packet hashlist hits the 25% dead threshold every ~25 packets. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
attermann
added a commit
that referenced
this pull request
Nov 10, 2026
Follow-up to #6. compact() now flushes and closes the active segment itself, runs the compaction body (moved to compact_segments()), and reopens min(current_segment, segment_count - 1) on every exit path. Fixes three issues in the #6 approach: - a failed rotation-triggered compaction created a stray seg<segment_count>.dat that clear() and finalize never remove - the early return when compact.tmp cannot be opened left the store with no active segment, so threshold-triggered and direct callers failed every later write until reboot - a failed rename in finalize_compaction() could reopen an out-of-range segment This also covers the threshold-path fix proposed in #11. Power loss: the flush and close happen before the COMPACTING journal is written, and the reopen happens after the journal is cleared (or deliberately retained for boot-time recovery). No segment writes occur in between, so crash recovery is unchanged. Adds RAM-fs open/write failure injection and tests for direct, threshold and rotation compaction failures.
Owner
|
Thanks for tracking this down, and especially for verifying it on a Heltec V4. Your explanation of why it only shows up on ESP-IDF LittleFS made the root cause easy to confirm. I'm closing this as superseded. #6 moved the close into compact() itself, so every caller is covered, including the threshold path you fixed and direct compact() calls. My follow-up commit 84e09a6 adds what your PR had and #6 didn't: a flush_buffer() before the close, and a reopen when compaction fails, so the threshold path keeps writing after a failed or aborted compaction. It also adds a native test for that threshold failure case. If you're able to re-check on your microReticulum setup against master, I'd welcome confirmation that the "Has open FD" errors are gone. Thanks again. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
compact_if_threshold()callscompact()whileactive_fileis still open. The rotation path (rotate_segment_if_needed()) closes the active file before compacting, but the threshold path does not.On ESP-IDF LittleFS, unlinking an open file is refused, so every threshold-triggered compaction fails to delete the active segment and logs:
twice per compaction (once in
compact()'s per-segment delete, once infinalize_compaction()). Plain POSIX permits unlinking an open file, which is why this doesn't reproduce off-device.Change
In
compact_if_threshold():flush_buffer(), closeactive_file, thencompact(). On success open seg1 as before; on failure or skip, reopencurrent_segmentso appends continue. Ten lines, mirrors the existing rotation path.Where it was seen
microReticulum_Firmware on a Heltec WiFi LoRa 32 V4 relaying link traffic between LoRa and a TCP interface. microReticulum's packet hashlist store defaults to 100 records, so with the 25% dead-record threshold the store compacts every ~25 forwarded packets, and each one hit this. Verified the error stops with the patch applied to the PlatformIO libdeps copy.
🤖 Generated with Claude Code