Skip to content

fix(journal): propagate purge close errors - #527

Merged
singaraiona merged 1 commit into
RayforceDB:devfrom
belowzeroff:fix/journal-purge-close-errors
Sep 14, 2026
Merged

singaraiona merged 1 commit into
RayforceDB:devfrom
belowzeroff:fix/journal-purge-close-errors

Conversation

@belowzeroff

@belowzeroff belowzeroff commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

What is wrong

.log.purge closes the active journal before removing the journal footprint, but the old code discards both return values:

if (g_journal.fp) {
    fflush(g_journal.fp);
    fclose(g_journal.fp);
    g_journal.fp = NULL;
}
/* .log, .qdb, and rolled archives are removed below */

If the final flush fails, for example because the filesystem reports ENOSPC or another I/O error while buffered journal data is being written, ray_journal_purge() continues anyway. The active .log, snapshot, and rolled archives can then be deleted even though the last journal writes were never confirmed as durable.

This is especially dangerous because purge is destructive: after it returns, the journal files may be gone and the failed buffered data may have been the only recoverable copy.

What changes

ray_journal_purge() now delegates closing to ray_journal_close(), which already checks both fflush() and fclose() and maps either failure to RAY_ERR_IO. On an error, purge returns immediately and leaves the journal files untouched. On success, it keeps the existing purge behavior and removes the complete journal footprint.

if (g_journal.fp) {
    ray_err_t close_result = ray_journal_close();
    if (close_result != RAY_OK) return close_result;
}

Validation

  • ./rayforce.test --filter journal/purge_: 3/3 passed
  • make test: compilation succeeded; 3647/3780 tests passed before the run was stopped after environment-dependent local-socket failures

@belowzeroff
belowzeroff force-pushed the fix/journal-purge-close-errors branch from 25ddd57 to 78c1713 Compare September 13, 2026 17:34

@singaraiona singaraiona left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against dev. The change is correct and I'd merge it.

  • .log.purge used to call fflush and fclose on the active journal, ignore both results, and then delete every journal file. A buffered write failing on the final flush (say ENOSPC) was silently dropped and the only copy of that data unlinked.
  • Delegating to ray_journal_close fixes that in the right place: it already checks both results, maps a failure to RAY_ERR_IO, and nulls g_journal.fp even on failure. Purge now returns before touching any file. Because the pointer is nulled either way, a retried purge cannot double-close, and g_journal.base survives the close, so a later purge still knows what to delete. Success-path behaviour is unchanged.

One thing to fix in the description before merging: "Fixes #60" points at a closed, unrelated segfault report (issue 60 is "SIGSEGV on try block"). Please drop that line or point it at the right issue, otherwise the merge annotates the wrong one.

https://claude.ai/code/session_017fCcYRuzNagw6nmLqZ9zbU

@singaraiona
singaraiona merged commit 611b576 into RayforceDB:dev Sep 14, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants