Skip to content

Conversation

@onopche
Copy link

@onopche onopche commented Dec 9, 2025

+Added FDB_TSDB_CTRL_SET_FORMAT_BAD_SEC_ONLY setting to TSDB to format only bad sectors to prevent data loss

Just a follow up on #383 issue. I'd imagine logic like this roughly. What it does is that rather than formatting all sectors it targets only affected sectors and carries on.

@onopche onopche changed the title Format bad sectors only tsdb: Format bad sectors only Dec 9, 2025
@onopche onopche force-pushed the format_bad_sectors_only branch from e807513 to f9a3820 Compare December 14, 2025 20:43
@armink armink requested a review from Copilot December 15, 2025 10:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a new FDB_TSDB_CTRL_SET_FORMAT_BAD_SEC_ONLY control command to the time-series database (TSDB) that enables selective formatting of only bad sectors instead of formatting the entire database. This feature aims to prevent data loss by preserving good sectors when corruption is detected during database initialization.

Key changes:

  • Added FDB_TSDB_CTRL_SET_FORMAT_BAD_SEC_ONLY constant and format_bad_sec_only field to enable selective bad sector formatting
  • Implemented new format_cb callback function to format individual sectors
  • Added logic in fdb_tsdb_init to iteratively check and format only bad sectors when the feature is enabled

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
inc/fdb_def.h Added new control command constant FDB_TSDB_CTRL_SET_FORMAT_BAD_SEC_ONLY (0x0C) and format_bad_sec_only boolean field to fdb_db struct; reformatted alignment of control command definitions
src/fdb_tsdb.c Implemented format_cb callback for selective sector formatting; added control case handler for the new command; added iterative bad sector detection and formatting logic in fdb_tsdb_init

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@onopche onopche force-pushed the format_bad_sectors_only branch from 8782457 to 43febe1 Compare December 17, 2025 20:12
@onopche
Copy link
Author

onopche commented Dec 17, 2025

@armink thanks for the review! I've changed the logic a bit (essentially just copied the bad sector formatting logic over from fdb_tsdb.c for consistency), which should address the AI comments. Any chance you could re-review? Cheers!

@armink
Copy link
Owner

armink commented Dec 18, 2025

My main concern is: in extreme cases, if only one sector is damaged and formatted, will this prevent the entire database from querying and inserting data?

@onopche
Copy link
Author

onopche commented Dec 18, 2025

My main concern is: in extreme cases, if only one sector is damaged and formatted, will this prevent the entire database from querying and inserting data?

I don't believe so. My understanding that the sector will be erased and header will be written and marked as FDB_SECTOR_STORE_EMPTY so it shouldn't affect the rest of the sectors - that's the point of selective formatting.

AFAICS, it's exactly same logic in KVDB, right? That's where I copied it to TSDB:

format_sector(db, sector->addr, SECTOR_NOT_COMBINED);

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