TSDB feature: Added FDB_TSDB_FIXED_BLOB_SIZE compile-time option to reduce flash usage in fixed-size blob scenarios. #387

EzraZigenbine · 2025-12-15T09:33:05Z

When enabled, removes log_len and log_addr fields from log_idx_data structure, saving 8 bytes per TSL entry. The log address is instead calculated at runtime based on the TSL's position within the sector.

This optimization is ideal for applications logging fixed-size data (e.g., single temperature as float + timestamp) where the 8-byte overhead per entry becomes significant with large numbers of entries.

Changes:

Conditionally exclude log_len/log_addr from log_idx_data struct
Calculate log address dynamically in read_tsl() when enabled
Add strict size validation in tsl_append()

…educe Flash usage in fixed-size blob scenarios. When enabled, removes log_len and log_addr fields from log_idx_data structure, saving 8 bytes per TSL entry. The log address is instead calculated at runtime based on the TSL's position within the sector. This optimization is ideal for applications logging fixed-size data (e.g., single temperature as float + timestamp) where the 8-byte overhead per entry becomes significant with large numbers of entries. Changes: - Conditionally exclude log_len/log_addr from log_idx_data struct - Calculate log address dynamically in read_tsl() when enabled - Add strict size validation in tsl_append() - Maintain backward compatibility via conditional compilation

…equential timestamp mode, reducing flash usage. This mode eliminates per-entry timestamp storage by calculating timestamps on-the-fly, saving sizeof(fdb_time_t) bytes per TSL entry (4 bytes for uint32_t, 8 bytes for uint64_t) per TSL entry. Key changes: - Removes field from structure when FDB_TSDB_SEQ_MODE is defined - Timestamps are calculated as: sector.start_time + TSL_index_in_sector - In tsl_append(), time auto-increments as db->last_time + 1 (no get_time() call needed) - In read_tsl(), timestamp is computed from sector start time and TSL position - Iterators pass sector.start_time via tsl.time before calling read_tsl() - search_start_tsl_addr() receives sector_start_time as additional parameter in SEQ_MODE Trade-offs: - Constraint: Timestamps must be monotonically increasing by 1 Depends on armink#387

Copilot

Pull request overview

This PR introduces a compile-time optimization for TSDB to reduce flash usage in scenarios where all TSL entries are the same fixed size. When FDB_TSDB_FIXED_BLOB_SIZE is enabled, it removes the log_len and log_addr fields from the log_idx_data structure (saving 8 bytes per entry) and calculates the log address dynamically during reads based on the TSL's position within the sector.

Key changes:

Conditionally exclude log_len and log_addr fields from log_idx_data struct when FDB_TSDB_FIXED_BLOB_SIZE is defined
Implement dynamic log address calculation in read_tsl() based on TSL index position
Add strict size validation in tsl_append() to enforce fixed-size constraint

Reviewed changes

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

File	Description
src/fdb_tsdb.c	Core implementation: conditional struct fields, dynamic address calculation in read_tsl(), modified write_tsl() to use local log_addr variable, and added size validation in tsl_append()
inc/fdb_cfg_template.h	Configuration template: added commented-out FDB_TSDB_FIXED_BLOB_SIZE definition with documentation

Critical Issues Identified:

The change to log_idx_data struct size affects LOG_IDX_DATA_SIZE macro, which is used extensively for sector layout calculations throughout the codebase. This creates an incompatible on-flash format between compilation modes.
The flash write operation at line 335 uses sizeof(struct log_idx_data) which varies based on the compile-time flag, potentially writing incorrect data.
Multiple indentation inconsistencies (tabs vs spaces) throughout the new code.

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

Copilot · 2025-12-15T10:55:28Z

src/fdb_tsdb.c

+#ifdef FDB_TSDB_FIXED_BLOB_SIZE
+		tsl->log_len = FDB_TSDB_FIXED_BLOB_SIZE;
+		sector_addr = FDB_ALIGN_DOWN(tsl->addr.index, db_sec_size(db));
+		tsl_index_in_sector = (tsl->addr.index - sector_addr - SECTOR_HDR_DATA_SIZE) / LOG_IDX_DATA_SIZE;
+		tsl->addr.log = sector_addr + db_sec_size(db) - (tsl_index_in_sector + 1) * FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE);
+		tsl->time = idx.time;
+#else


Inconsistent indentation: These lines use tabs while the surrounding code uses spaces. Please use spaces for consistency with the rest of the file.

All code format needs to be checked.

Fixed see link

Copilot · 2025-12-15T10:55:29Z

src/fdb_tsdb.c

    uint32_t idx_addr = db->cur_sec.empty_idx;
-
+#ifdef FDB_TSDB_FIXED_BLOB_SIZE
+	 uint32_t log_addr = db->cur_sec.empty_data - FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE);


Inconsistent indentation: These lines use tabs while the surrounding code uses spaces. Please use spaces for consistency with the rest of the file.

Copilot · 2025-12-15T10:55:29Z

src/fdb_tsdb.c

+	if(blob->size != FDB_TSDB_FIXED_BLOB_SIZE) {
+		FDB_INFO("Error: blob size (%zu) must equal FDB_TSDB_FIXED_BLOB_SIZE (%d)\n", blob->size, FDB_TSDB_FIXED_BLOB_SIZE);
+		return FDB_WRITE_ERR;
+	}


Improved validation message: Consider adding the actual value of FDB_TSDB_FIXED_BLOB_SIZE to the error message for better debugging. Also, there's missing space after "if" which violates standard C style guidelines.

Suggested change

if(blob->size != FDB_TSDB_FIXED_BLOB_SIZE) {

FDB_INFO("Error: blob size (%zu) must equal FDB_TSDB_FIXED_BLOB_SIZE (%d)\n", blob->size, FDB_TSDB_FIXED_BLOB_SIZE);

return FDB_WRITE_ERR;

}

if (blob->size != FDB_TSDB_FIXED_BLOB_SIZE) {

FDB_INFO("Error: blob size (%zu) does not match required fixed size (%d)\n", blob->size, FDB_TSDB_FIXED_BLOB_SIZE);

return FDB_WRITE_ERR;

}

Copilot · 2025-12-15T10:55:29Z

src/fdb_tsdb.c

+#ifdef FDB_TSDB_FIXED_BLOB_SIZE
+	uint32_t tsl_index_in_sector;
+	uint32_t sector_addr;
+#endif


Inconsistent indentation: These lines use tabs while the surrounding code uses spaces. Please use spaces for consistency with the rest of the file.

Copilot · 2025-12-15T10:55:29Z

src/fdb_tsdb.c

    FLASH_WRITE(db, idx_addr + LOG_IDX_TS_OFFSET, &idx.time,  sizeof(struct log_idx_data) - LOG_IDX_TS_OFFSET, false);
    /* write blob data */
+#ifdef FDB_TSDB_FIXED_BLOB_SIZE
+	FLASH_WRITE(db, log_addr, blob->buf, blob->size, false);


Inconsistent indentation: These lines use tabs while the surrounding code uses spaces. Please use spaces for consistency with the rest of the file.

Suggested change

FLASH_WRITE(db, log_addr, blob->buf, blob->size, false);

FLASH_WRITE(db, log_addr, blob->buf, blob->size, false);

Copilot · 2025-12-15T10:55:30Z

src/fdb_tsdb.c

+#ifdef FDB_TSDB_FIXED_BLOB_SIZE
+	if(blob->size != FDB_TSDB_FIXED_BLOB_SIZE) {
+		FDB_INFO("Error: blob size (%zu) must equal FDB_TSDB_FIXED_BLOB_SIZE (%d)\n", blob->size, FDB_TSDB_FIXED_BLOB_SIZE);
+		return FDB_WRITE_ERR;
+	}


Inconsistent indentation: These lines use tabs while the surrounding code uses spaces. Please use spaces for consistency with the rest of the file.

Copilot · 2025-12-15T10:55:30Z

src/fdb_tsdb.c

    /* write the status will by write granularity */
    _FDB_WRITE_STATUS(db, idx_addr, idx.status_table, FDB_TSL_STATUS_NUM, FDB_TSL_PRE_WRITE, false);
    /* write other index info */
    FLASH_WRITE(db, idx_addr + LOG_IDX_TS_OFFSET, &idx.time,  sizeof(struct log_idx_data) - LOG_IDX_TS_OFFSET, false);


Critical bug: When FDB_TSDB_FIXED_BLOB_SIZE is defined, the log_idx_data structure no longer contains log_len and log_addr fields. This write operation attempts to write "sizeof(struct log_idx_data) - LOG_IDX_TS_OFFSET" bytes from &idx.time, which would only write the time field. However, in the non-fixed-size case, this writes time, log_len, and log_addr. The size calculation needs to be conditional on FDB_TSDB_FIXED_BLOB_SIZE to ensure the correct number of bytes are written.

Suggested change

FLASH_WRITE(db, idx_addr + LOG_IDX_TS_OFFSET, &idx.time, sizeof(struct log_idx_data) - LOG_IDX_TS_OFFSET, false);

#ifdef FDB_TSDB_FIXED_BLOB_SIZE

FLASH_WRITE(db, idx_addr + LOG_IDX_TS_OFFSET, &idx.time, sizeof(idx.time), false);

#else

FLASH_WRITE(db, idx_addr + LOG_IDX_TS_OFFSET, &idx.time, sizeof(struct log_idx_data) - LOG_IDX_TS_OFFSET, false);

#endif

This AI comments can be ignored

Copilot · 2025-12-15T10:55:30Z

src/fdb_tsdb.c

+#ifndef FDB_TSDB_FIXED_BLOB_SIZE
    uint32_t log_len;                            /**< node total length (header + name + value), must align by FDB_WRITE_GRAN */
    uint32_t log_addr;                           /**< node address */
+#endif


Critical design issue: LOG_IDX_DATA_SIZE is defined as FDB_WG_ALIGN(sizeof(struct log_idx_data)) and is used throughout the codebase for sector layout calculations, address computations, and space accounting. When FDB_TSDB_FIXED_BLOB_SIZE is enabled, the struct size changes (removing 8 bytes), which changes LOG_IDX_DATA_SIZE. This creates an incompatible on-flash format - databases created with this feature enabled cannot be read by code compiled without it, and vice versa. This breaks the entire storage layout and makes the feature fundamentally incompatible with existing data. Consider using a separate macro or ensuring the struct size remains constant to maintain format compatibility.

Suggested change

#ifndef FDB_TSDB_FIXED_BLOB_SIZE

uint32_t log_len; /**< node total length (header + name + value), must align by FDB_WRITE_GRAN */

uint32_t log_addr; /**< node address */

#endif

uint32_t log_len; /**< node total length (header + name + value), must align by FDB_WRITE_GRAN */

uint32_t log_addr; /**< node address */

Ignoring the AI's comment for now, enabling this macro will indeed cause incompatibility in the data format stored in Flash. Just specify this when configuring the macro.

updated see link

Copilot · 2025-12-15T10:55:31Z

src/fdb_tsdb.c

+		tsl_index_in_sector = (tsl->addr.index - sector_addr - SECTOR_HDR_DATA_SIZE) / LOG_IDX_DATA_SIZE;
+		tsl->addr.log = sector_addr + db_sec_size(db) - (tsl_index_in_sector + 1) * FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE);


Potential calculation error: The calculation of tsl_index_in_sector divides by LOG_IDX_DATA_SIZE, but LOG_IDX_DATA_SIZE itself changes when FDB_TSDB_FIXED_BLOB_SIZE is enabled (smaller struct). This means the index calculation will be correct for the current compilation mode, but the data layout on flash will be incompatible with code compiled in a different mode. Additionally, verify that the log address calculation correctly accounts for alignment - it assumes entries are packed from the end of the sector backwards with FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE) spacing, which should match how write_tsl() allocates them.

armink · 2025-12-16T01:34:11Z

src/fdb_tsdb.c

    idx.log_len = blob->size;
-    idx.time = time;
    idx.log_addr = db->cur_sec.empty_data - FDB_WG_ALIGN(idx.log_len);
+#endif


#ifdef FDB_TSDB_FIXED_BLOB_SIZE uint32_t log_addr = db->cur_sec.empty_data - FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE); uint32_t log_len = FDB_TSDB_FIXED_BLOB_SIZE; #else uint32_t log_addr = db->cur_sec.empty_data - FDB_WG_ALIGN(idx.log_len); uint32_t log_len = blob->size; #endif

With this code modification, the following code should have higher reusability.

Fixed.
As I understand the request, this update improves reusability of the flash write calls later in the function by removing #ifdef.
Instead of the suggested approach, used blob->size, which is already fixed or variable depending on the macro.

…ash data format, as requested in PR review

armink · 2025-12-17T01:04:30Z

src/fdb_tsdb.c

        tsl->addr.log = FDB_DATA_UNUSED;
        tsl->time = 0;
    } else {
+#ifdef FDB_TSDB_FIXED_BLOB_SIZE


this code can move to here

uint32_t tsl_index_in_sector; uint32_t sector_addr;

Copilot

Pull request overview

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

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

Copilot · 2025-12-17T01:18:13Z

inc/fdb_cfg_template.h

+/* Use fixed-size blobs in TSDB to save flash overhead (8 bytes per entry).
+ * Define this to the fixed blob size in bytes when all TSL entries are the same size.
+ * Ideal for logging fixed-size sensor data (e.g., float + timestamp).
+ * Warning: If defined will be incompataible with variable blob flash store or if fix blob size is later changed */


Spelling error: "incompataible" should be "incompatible".

Copilot · 2025-12-17T01:18:13Z

src/fdb_tsdb.c

    fdb_time_t cur_time = timestamp == NULL ? db->get_time() : *timestamp;

+#ifdef FDB_TSDB_FIXED_BLOB_SIZE
+    if(blob->size != FDB_TSDB_FIXED_BLOB_SIZE) {


Missing space after 'if' keyword. C style conventions typically require a space between 'if' and the opening parenthesis for better readability.

Copilot · 2025-12-17T01:18:14Z

inc/fdb_cfg_template.h

+/* Use fixed-size blobs in TSDB to save flash overhead (8 bytes per entry).
+ * Define this to the fixed blob size in bytes when all TSL entries are the same size.
+ * Ideal for logging fixed-size sensor data (e.g., float + timestamp).
+ * Warning: If defined will be incompataible with variable blob flash store or if fix blob size is later changed */


Spelling error: "fix" should be "fixed" (as in "fixed blob size").

Copilot · 2025-12-17T01:18:14Z

src/fdb_tsdb.c

+#ifdef FDB_TSDB_FIXED_BLOB_SIZE
+    if(blob->size != FDB_TSDB_FIXED_BLOB_SIZE) {
+        FDB_INFO("Error: blob size (%zu) must equal FDB_TSDB_FIXED_BLOB_SIZE (%d)\n", blob->size, FDB_TSDB_FIXED_BLOB_SIZE);
+        return FDB_WRITE_ERR;
+    }
+#else
    /* check the append length, MUST less than the db->max_len */
    if(blob->size > db->max_len)
    {
        FDB_INFO("Warning: append length (%" PRIdMAX ") is more than the db->max_len (%" PRIdMAX "). This tsl will be dropped.\n", 
                (intmax_t)blob->size, (intmax_t)(db->max_len));
        return FDB_WRITE_ERR;
    }
+#endif


The new FDB_TSDB_FIXED_BLOB_SIZE feature lacks test coverage. Consider adding tests to verify the fixed-size blob validation logic, the runtime address calculation, and the interaction with existing TSDB operations.

armink · 2025-12-17T01:22:39Z

src/fdb_tsdb.c

+        tsl->log_len = FDB_TSDB_FIXED_BLOB_SIZE;
+        sector_addr = FDB_ALIGN_DOWN(tsl->addr.index, db_sec_size(db));
+        tsl_index_in_sector = (tsl->addr.index - sector_addr - SECTOR_HDR_DATA_SIZE) / LOG_IDX_DATA_SIZE;
+        tsl->addr.log = sector_addr + db_sec_size(db) - (tsl_index_in_sector + 1) * FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE);


@copilot Is there a way to simplify the logic here?

This regression was introduced by previous commit 0d3899a.

armink · 2025-12-17T11:25:46Z

Thx @EzraZigenbine

EzraZigenbine mentioned this pull request Dec 15, 2025

TSDB feature: Added FDB_TSDB_SEQ_MODE compile-time option to enable sequential timestamp mode, reducing flash usage. #388

Open

armink requested a review from Copilot December 15, 2025 10:51

Copilot started reviewing on behalf of armink December 15, 2025 10:52 View session

Copilot AI reviewed Dec 15, 2025

View reviewed changes

armink reviewed Dec 16, 2025

View reviewed changes

EzraZigenbine added 3 commits December 16, 2025 20:09

Fix inconsistent indentation noted in PR review

a32bcba

Add warning that enabling this macro will cause incompatibility in Fl…

b2bd249

…ash data format, as requested in PR review

Improve FLASH_WRITE reusability as requested in PR review

0d3899a

armink reviewed Dec 17, 2025

View reviewed changes

armink requested a review from Copilot December 17, 2025 01:14

Copilot started reviewing on behalf of armink December 17, 2025 01:14 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

armink reviewed Dec 17, 2025

View reviewed changes

EzraZigenbine added 3 commits December 17, 2025 19:33

tsdb: fix missing log address/size for variable blobs

6dcd4ad

This regression was introduced by previous commit 0d3899a.

Move fixed-size variables to reduce #ifdef usage (pre-PR review)

dc35884

fix spelling and formatting per PR review

cf3275c

armink merged commit 8856961 into armink:master Dec 17, 2025
2 checks passed

	FLASH_WRITE(db, log_addr, blob->buf, blob->size, false);
	FLASH_WRITE(db, log_addr, blob->buf, blob->size, false);

		tsl_index_in_sector = (tsl->addr.index - sector_addr - SECTOR_HDR_DATA_SIZE) / LOG_IDX_DATA_SIZE;
		tsl->addr.log = sector_addr + db_sec_size(db) - (tsl_index_in_sector + 1) * FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE);

TSDB feature: Added FDB_TSDB_FIXED_BLOB_SIZE compile-time option to reduce flash usage in fixed-size blob scenarios. #387

TSDB feature: Added FDB_TSDB_FIXED_BLOB_SIZE compile-time option to reduce flash usage in fixed-size blob scenarios. #387

Uh oh!

Conversation

EzraZigenbine commented Dec 15, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

armink Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EzraZigenbine Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

armink Dec 16, 2025 •

edited

Loading

EzraZigenbine Dec 16, 2025 •

edited

Loading