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

EzraZigenbine · 2025-12-15T09:55:11Z

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 #387

…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 pull request introduces a compile-time sequential timestamp mode (FDB_TSDB_USING_SEQ_MODE) for the Time Series Database (TSDB) feature to reduce flash storage requirements. The mode eliminates per-entry timestamp storage by calculating timestamps on-the-fly based on sector start time and TSL index position, saving sizeof(fdb_time_t) bytes per entry (4-8 bytes depending on configuration).

Key changes:

Conditionally removes the time field from log_idx_data structure and get_time function pointer from fdb_tsdb structure when sequential mode is enabled
Implements timestamp calculation as sector.start_time + TSL_index_in_sector in read operations
Auto-increments timestamps as db->last_time + 1 in append operations without requiring time function calls
Updates all TSL iterators and search functions to pass sector start time for timestamp calculation

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 18 comments.

File	Description
inc/fdb_cfg_template.h	Adds configuration template and documentation for the new `FDB_TSDB_USING_SEQ_MODE` and `FDB_TSDB_FIXED_BLOB_SIZE` compile-time options
inc/fdb_def.h	Conditionally removes `get_time` function pointer from `fdb_tsdb` structure when sequential mode is enabled
src/fdb_tsdb.c	Core implementation of sequential timestamp mode including modified read/write functions, timestamp calculation logic, iterator updates, and conditional compilation throughout

Comments suppressed due to low confidence (1)

src/fdb_tsdb.c:464

In FDB_TSDB_USING_SEQ_MODE, cur_time is always set to db->last_time + 1, which means the timestamp validation check on line 460 (cur_time <= db->last_time) will always evaluate to false and pass. This check is redundant in sequential mode and should be conditionally compiled out to avoid unnecessary overhead and misleading code. Consider wrapping the timestamp validation check (lines 460-464) with #ifndef FDB_TSDB_USING_SEQ_MODE to skip it when sequential mode is enabled.

    /* check the current timestamp, MUST more than the last save timestamp */
    if (cur_time <= db->last_time) {
        FDB_INFO("Warning: current timestamp (%" PRIdMAX ") is less than or equal to the last save timestamp (%" PRIdMAX "). This tsl will be dropped.\n",
                (intmax_t )cur_time, (intmax_t )(db->last_time));
        return FDB_WRITE_ERR;
    }

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

Copilot · 2025-12-15T10:43:20Z

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: this line uses tabs while the surrounding code uses spaces. This should be aligned with the project's indentation style.

Suggested change

uint32_t log_addr = db->cur_sec.empty_data - FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE);

uint32_t log_addr = db->cur_sec.empty_data - FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE);

Copilot · 2025-12-15T10:43:20Z

src/fdb_tsdb.c

+#ifndef FDB_TSDB_FIXED_BLOB_SIZE
+    FLASH_WRITE(db, idx_addr + LOG_IDX_TS_OFFSET, &idx.log_len,  sizeof(struct log_idx_data) - LOG_IDX_TS_OFFSET, false);
+#endif
+// #else no other index info


Inconsistent indentation: this line uses a leading space followed by a tab, while the project appears to use consistent spacing. This should be aligned with the project's indentation style.

Suggested change

// #else no other index info

// #else no other index info

Copilot · 2025-12-15T10:43:21Z

src/fdb_tsdb.c

                /* search all TSL */
                do {
+#ifdef FDB_TSDB_USING_SEQ_MODE
+					tsl.time = sector.start_time;


Inconsistent indentation: this line uses tabs while the surrounding code uses spaces. This should be aligned with the project's indentation style.

Suggested change

tsl.time = sector.start_time;

tsl.time = sector.start_time;

Copilot · 2025-12-15T10:43:21Z

src/fdb_tsdb.c

+#if defined(FDB_TSDB_FIXED_BLOB_SIZE) || defined(FDB_TSDB_USING_SEQ_MODE)
+	uint32_t tsl_index_in_sector;
+	uint32_t sector_addr;


The condition here combines two independent features (FDB_TSDB_FIXED_BLOB_SIZE and FDB_TSDB_USING_SEQ_MODE) but the variable declarations are only needed for their respective features. The tsl_index_in_sector and sector_addr variables should be declared in separate conditional blocks to avoid unnecessarily declaring them when only one feature is enabled. Consider splitting this into two separate conditional blocks or using a logical OR if both features need these variables.

Suggested change

#if defined(FDB_TSDB_FIXED_BLOB_SIZE) || defined(FDB_TSDB_USING_SEQ_MODE)

uint32_t tsl_index_in_sector;

uint32_t sector_addr;

#ifdef FDB_TSDB_USING_SEQ_MODE

uint32_t tsl_index_in_sector;

#endif

#if defined(FDB_TSDB_FIXED_BLOB_SIZE) || defined(FDB_TSDB_USING_SEQ_MODE)

uint32_t sector_addr;

Copilot · 2025-12-15T10:43:21Z

src/fdb_tsdb.c

+#ifdef FDB_TSDB_USING_SEQ_MODE
+#define LOG_IDX_TS_OFFSET                        ((unsigned long)(&((struct log_idx_data *)0)->log_len))
+#else
 #define LOG_IDX_TS_OFFSET                        ((unsigned long)(&((struct log_idx_data *)0)->time))
+#endif


When FDB_TSDB_USING_SEQ_MODE is defined, LOG_IDX_TS_OFFSET is set to the offset of log_len field in the log_idx_data structure. However, the log_len field only exists when FDB_TSDB_FIXED_BLOB_SIZE is NOT defined (see lines 103-106). This means if both FDB_TSDB_USING_SEQ_MODE and FDB_TSDB_FIXED_BLOB_SIZE are defined together, LOG_IDX_TS_OFFSET would reference a non-existent field, leading to incorrect offset calculations and potential memory corruption. Consider adding a compile-time check to prevent these two options from being enabled simultaneously, or adjust the logic to handle this combination properly.

Copilot · 2025-12-15T10:43:24Z

src/fdb_tsdb.c

+#ifndef FDB_TSDB_FIXED_BLOB_SIZE
+    FLASH_WRITE(db, idx_addr + LOG_IDX_TS_OFFSET, &idx.log_len,  sizeof(struct log_idx_data) - LOG_IDX_TS_OFFSET, false);
+#endif
+// #else no other index info


The comment "// #else no other index info" is confusing and appears to be a note that shouldn't be in the final code. If the intent is to document that no flash write is needed when both FDB_TSDB_USING_SEQ_MODE and FDB_TSDB_FIXED_BLOB_SIZE are defined, this should be expressed as a proper comment or removed entirely. Consider either removing this line or replacing it with a clearer comment like "/* No other index info to write when both SEQ_MODE and FIXED_BLOB_SIZE are enabled */".

Suggested change

// #else no other index info

/* No other index info to write when both SEQ_MODE and FIXED_BLOB_SIZE are enabled */

Copilot · 2025-12-15T10:43:25Z

inc/fdb_cfg_template.h

+/* #define FDB_TSDB_FIXED_BLOB_SIZE 4 */
+
+
+/* Use sequentual mode - eliminates per-entry timestamp storage by calculating timestamps


Spelling error: "sequentual" should be "sequential".

Suggested change

/* Use sequentual mode - eliminates per-entry timestamp storage by calculating timestamps

/* Use sequential mode - eliminates per-entry timestamp storage by calculating timestamps

Copilot · 2025-12-15T10:43:25Z

inc/fdb_cfg_template.h

+/* Use sequentual mode - eliminates per-entry timestamp storage by calculating timestamps
+ * on-the-fly, saving sizeof(fdb_time_t). In this mode timestamps are monotonically increasing by 1 on each log
+ * and don't require application to provide a time function.


The comment claims this saves "sizeof(fdb_time_t)" but the actual savings depends on whether FDB_TSDB_FIXED_BLOB_SIZE is also defined. Without FDB_TSDB_FIXED_BLOB_SIZE defined, the structure still contains log_len and log_addr fields. The comment should clarify that the savings apply to the timestamp field specifically, but the overall structure size reduction depends on other compile-time options.

Suggested change

/* Use sequentual mode - eliminates per-entry timestamp storage by calculating timestamps

* on-the-fly, saving sizeof(fdb_time_t). In this mode timestamps are monotonically increasing by 1 on each log

* and don't require application to provide a time function.

/* Use sequential mode - eliminates per-entry timestamp storage by calculating timestamps

* on-the-fly, saving the space used for the timestamp field (sizeof(fdb_time_t)) per entry.

* Note: The overall structure size reduction depends on other compile-time options (e.g., FDB_TSDB_FIXED_BLOB_SIZE).

* In this mode, timestamps are monotonically increasing by 1 on each log and don't require the application to provide a time function.

Copilot · 2025-12-15T10:43:25Z

src/fdb_tsdb.c

+	fdb_time_t cur_time = db->last_time + 1;
+	(void)timestamp;
+#else
    fdb_time_t cur_time = timestamp == NULL ? db->get_time() : *timestamp;
+#endif

+#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: this line uses tabs while the surrounding code uses spaces. This should be aligned with the project's indentation style.

Suggested change

fdb_time_t cur_time = db->last_time + 1;

(void)timestamp;

#else

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

#endif

#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;

}

fdb_time_t cur_time = db->last_time + 1;

(void)timestamp;

#else

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

#endif

#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;

}

Copilot · 2025-12-15T10:43:26Z

src/fdb_tsdb.c

            if (from > to) {
                tsl.addr.index = start;
+#ifdef FDB_TSDB_USING_SEQ_MODE
+				tsl.time = start_time;


Inconsistent indentation: this line uses tabs while the surrounding code uses spaces. This should be aligned with the project's indentation style.

Suggested change

tsl.time = start_time;

tsl.time = start_time;

armink · 2025-12-16T01:41:33Z

Thx for your PR. We might have to wait until #387 is merged before reviewing #388; the differences will likely be smaller then.

…ash data format, as requested in PR review

This regression was introduced by previous commit 0d3899a.

armink · 2025-12-18T00:48:04Z

@EzraZigenbine can you fix the conflicts?

armink · 2025-12-20T14:12:12Z

There are still conflicts that need to be resolved here

EzraZigenbine · 2025-12-21T09:52:36Z

fixed conflicts

armink · 2025-12-23T00:38:03Z

inc/fdb_def.h

    fdb_time_t last_time;                        /**< last TSL timestamp */
+#ifndef FDB_TSDB_USING_SEQ_MODE
    fdb_get_time get_time;                       /**< the current timestamp get function */
+#endif


For sequential mode, isn't that simpler?

Do not allow users to set get_time functions from the outside, the system forces the default get_time function, and users will be prompted with an error when setting it

typedef fdb_time_t (*fdb_get_time)(void); -> typedef fdb_time_t (*fdb_get_time)(fdb_tsdb_t db);

The default get_time function of the system only needs to implement db->last_time +1 and then return this time number

Since FDB_TSDB_USING_SEQ_MODE is a compile-time option, my intention was to prevent misconfiguration at compile time rather than failing at runtime.

While compiling out get_time from the struct enforces this internally, I should also update the function signature to produce a compile-time error if an application attempts to pass a get_time function in SEQ mode, e.g.:

#ifdef FDB_TSDB_USING_SEQ_MODE
fdb_err_t fdb_tsdb_init(fdb_tsdb_t db, const char *name, const char *path, size_t max_len, void *user_data);
#else
fdb_err_t fdb_tsdb_init(fdb_tsdb_t db, const char *name, const char *path, db_get_time get_time, size_t max_len, void *user_data);
#endif

Also should have compiled out fdb_tsl_append_with_ts for sequential mode as it should never be called.

Enforcing this at runtime would be simpler, but it can result in delayed detection of misconfiguration.

Alternatively can see advantage of keeping consistent API and db struct. (simplifies code and maintenance). and error when setting get_time is more consistent with how fix blob work i.e. get run time error if not expected fixed size.

ok to update ether as suggested or add more compile time gards. let me know.

in above suggestion would this change:

typedef fdb_time_t (*fdb_get_time)(void); -> typedef fdb_time_t (*fdb_get_time)(fdb_tsdb_t db);

be for all builds or just seq mode?

Or you can do this, fdb_tsdb_init when initializing, the get_time parameter is empty, then the sequential mode is used by default, so that the initialization document needs to be updated clearly

fdb_tsl_append_with_ts Make assertions in this sequential mode

A v2.2 version can be released later, just explain the incompatibility clearly, fdb_get_time it is still necessary to modify, including fdb_tsl_cb I hope to change it as well to add fdb parameters

EzraZigenbine added 2 commits December 15, 2025 19:22

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

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

Copilot AI reviewed Dec 15, 2025

View reviewed changes

EzraZigenbine added 7 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

Merge branch 'feature_fix_blob' into feature_seq_mode

2557316

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

EzraZigenbine added 3 commits December 18, 2025 21:37

Merge branch 'feature_fix_blob' into feature_seq_mode

91be6c7

Fixed formatting

5c5c31e

Fixed spelling and added warning comment for sequential mode

a781eb4

Merge branch 'master' into feature_seq_mode

ecc3722

armink reviewed Dec 23, 2025

View reviewed changes

	uint32_t log_addr = db->cur_sec.empty_data - FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE);
	uint32_t log_addr = db->cur_sec.empty_data - FDB_WG_ALIGN(FDB_TSDB_FIXED_BLOB_SIZE);

	// #else no other index info
	/* No other index info to write when both SEQ_MODE and FIXED_BLOB_SIZE are enabled */

		/* #define FDB_TSDB_FIXED_BLOB_SIZE 4 */


		/* Use sequentual mode - eliminates per-entry timestamp storage by calculating timestamps

	/* Use sequentual mode - eliminates per-entry timestamp storage by calculating timestamps
	/* Use sequential mode - eliminates per-entry timestamp storage by calculating timestamps

-/* Use sequentual mode - eliminates per-entry timestamp storage by calculating timestamps
- * on-the-fly, saving sizeof(fdb_time_t). In this mode timestamps are monotonically increasing by 1 on each log
- * and don't require application to provide a time function.
+/* Use sequential mode - eliminates per-entry timestamp storage by calculating timestamps
+ * on-the-fly, saving the space used for the timestamp field (sizeof(fdb_time_t)) per entry.
+ * Note: The overall structure size reduction depends on other compile-time options (e.g., FDB_TSDB_FIXED_BLOB_SIZE).
+ * In this mode, timestamps are monotonically increasing by 1 on each log and don't require the application to provide a time function.

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

Are you sure you want to change the base?

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

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!

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!

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!

armink commented Dec 16, 2025

Uh oh!

armink commented Dec 18, 2025

Uh oh!

armink commented Dec 20, 2025

Uh oh!

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

Uh oh!

Uh oh!

armink Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

EzraZigenbine Dec 24, 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!

armink Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

EzraZigenbine commented Dec 21, 2025 •

edited

Loading

EzraZigenbine Dec 24, 2025 •

edited

Loading