tsdb: Format bad sectors only

[onopche]

+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.

[Copilot]

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]

@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]

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]

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:

FlashDB/src/fdb_kvdb.c

Line 1599 in 8856961

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

[armink]

KVDB does not require the order of Flash sector addresses, but TSDB does.

[onopche]

KVDB does not require the order of Flash sector addresses, but TSDB does.

Hey, @armink sorry for a belated reply, was on a break last 2 weeks.

Sure, but TSDB has enough info (sector size) to carry on even if a sector is damaged/erased. I don't quite understand how it supposed to work with regular TSDB updates and random power offs - my understanding, with the current logic it's a very high chance of losing all data on every power off, isn't it (append request -> erase sector -> power off (before we had a chance to update the header))?

[armink]

KVDB does not require the order of Flash sector addresses, but TSDB does.

Hey, @armink sorry for a belated reply, was on a break last 2 weeks.

Sure, but TSDB has enough info (sector size) to carry on even if a sector is damaged/erased. I don't quite understand how it supposed to work with regular TSDB updates and random power offs - my understanding, with the current logic it's a very high chance of losing all data on every power off, isn't it (append request -> erase sector -> power off (before we had a chance to update the header))?

Because each sector header in TSDB stores start and end timestamp information, if the timestamp information for a sector is missing, the time-based search function may fail.

[onopche]

Because each sector header in TSDB stores start and end timestamp information, if the timestamp information for a sector is missing, the time-based search function may fail.

Sure, but it's still a better option than losing all data, right? For me, it's choice between "may fail" and "will always fail". If it's a concern, I could put this logic behind FDB_TSDB_CTRL_ setting (e.g. FDB_TSDB_CTRL_FORMAT_BAD_SEC_ONLY), the way I did it before:

#define FDB_TSDB_CTRL_SET_FORMAT_BAD_SEC_ONLY   0x0C    /**< set format bad sectors only mode control command, this change MUST before database initialization */

and default it to off for those who prefer the existing logic.