Skip to content

feat(core): add audit history journal backfill change - #982

Open
bercianor wants to merge 2 commits into
masterfrom
feat/journal-backfill
Open

bercianor wants to merge 2 commits into
masterfrom
feat/journal-backfill

Conversation

@bercianor

Copy link
Copy Markdown
Contributor

Summary

  • Add JournalBackfillChange to copy historical audit entries into journal streams without modifying audit history; registration and execution ordering remain outside this change.
  • Preserve history order and source payloads, reuse persistence instances by stage, and exclude the backfill change's own audit records.
  • Require journal events to be enabled and stop on unsuccessful appends; document that retries with a fresh instance may duplicate earlier entries.

Testing

  • Added six unit tests covering feature gating, empty history, payload and ordering preservation, stage-specific persistence reuse, append failures, and retry behavior.
  • Tests were not rerun for this PR; automated validation remains pending CI.


logger.info("Backfilling audit history into journal streams");
this.persistenceFactory = persistenceFactory;
for (AuditEntry auditEntry : persistenceFactory.get(SYSTEM_STAGE_ID).getAuditHistory()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor, not blocking: this read is not going through the journalPersistenceByStage cache.

Simple version: this line asks the factory directly for the SYSTEM_STAGE_ID persistence (just to read history). If an audit entry later in the loop also has stageId == SYSTEM_STAGE_ID, getJournalPersistence (line 85) will call persistenceFactory.get(SYSTEM_STAGE_ID) again and build a second, separate persistence instance for appending — the one used here for reading is thrown away unused afterward.

Today this is harmless for Mongo (its sequencer re-reads the DB each time it is built, so no duplicate sequence numbers), just a wasted extra instantiation/initialize call. But it does contradict the javadoc above (journalBackfill) which says persistence instances are reused by stage — worth a comment or routing this first read through the same cache, in case a future backend's initialize() is not idempotent.

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