Skip to content

Temporary EOF Log Marker#535

Merged
gbin merged 3 commits into
copper-project:masterfrom
ishanchadha01:ishanchadha01/temp-end-marker
Nov 28, 2025
Merged

Temporary EOF Log Marker#535
gbin merged 3 commits into
copper-project:masterfrom
ishanchadha01:ishanchadha01/temp-end-marker

Conversation

@ishanchadha01

Copy link
Copy Markdown
Contributor
  • includes UnifiedLogger lib definition
  • implementation of temporary EOF marker that's overwritten when new log entry is written
  • tests for the temporary marker and permanent marker
  • mirrored functionality in SDLogger

@gbin

gbin commented Nov 27, 2025

Copy link
Copy Markdown
Collaborator

that's overwritten when new log entry is written <- more like new section is written?

let header = SectionHeader {
magic: SECTION_MAGIC,
block_size: SECTION_HEADER_COMPACT_SIZE,
entry_type: UnifiedLogType::LastEntry,

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.

Writing a new payload for basically a boolean is overkill.

I'd like to propose an alternative that might be better: as we write all the section header when we close them, we could add a "still open" boolean in all the section headers. The first time we create them we leave the boolean at open, then when we close them we flip the boolean (we write that sector anyway so it is almost free).

We use this flag on the temporary end marker to signal that as a while the entire log was not closed properly.

Like that the log reader can detect the still open / dirty sections when the log has not been closed properly.

Then we need to adapt the log reader and fsck to emit a warning that they stopped reading the log on a not cleanly closed log and list the sections that were still open (hinting to the user that they miss some data in there).

fn write_end_marker(&mut self, temporary: bool) -> CuResult<()> {
let marker = EndOfLogMarker { temporary };

let mut payload = [0u8; 32];

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.

I think this is overkill to create a payload for this. See below

@gbin

gbin commented Nov 27, 2025

Copy link
Copy Markdown
Collaborator

thanks for the PR, see inside but I think we would be better off if we add this boolean on every section header see the comments in the review.

@ishanchadha01

Copy link
Copy Markdown
Contributor Author

Sounds good - i tried refactoring to use a boolean, the logic does seem simpler now

@gbin

gbin commented Nov 28, 2025

Copy link
Copy Markdown
Collaborator

Thx

@gbin gbin merged commit ed6c638 into copper-project:master Nov 28, 2025
7 checks passed
joshuaPurushothaman pushed a commit to joshuaPurushothaman/copper-rs that referenced this pull request Dec 4, 2025
* lib definition, memmap impl and passing tests, sdlogger impl

* replace payload with bool
@makeecat makeecat added the enhancement New feature or request label Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants