search

armink / FlashDB

An ultra-lightweight database that supports key-value and time series data | 一款支持 KV 数据和时序数据的超轻量级数据库
Apache License 2.0
1.93k stars 435 forks source link

Question: any reason to check that new entry timestamp is newer than previous? #257

Open valkuc opened 1 year ago

valkuc commented 1 year ago

Hi. I'm referring to this line https://github.com/armink/FlashDB/blob/a208b1555b9805c4fb13336b0c1529d2df8d5641/src/fdb_tsdb.c#L398C6-L398C6

Any reason to perform this check? What bad can happen if user will be allowed to save TSDB entries with "out-of-order" timestamp?

I'm just thinking about situation when having this check can break TSDB data appending. Example: 1) Time: 01-01-2023 - add new entry - OK 2) Time: 02-01-2023 - add new entry - OK 3) Time: 03-03-2023 (user mistakenly changed the date in the system) - add new entry - FAIL 4) Time: 03-01-2023 (user fixed the date in the system) - add new entry - FAIL 5) and it will always fail until event date will be after 03-03-2023

UPD: I'm actually faced such situations while developing firmware, but in my case (time_t is int64) I got a saved event somewhere in the year 32012 :-)

armink commented 1 year ago

Because the FlashDB uses the binary search algorithm to search TSDB. The search will cause problems when timestamp cannot keep incrementing.

valkuc commented 1 year ago

If the binary search is used just for iteration functions then what about make this feature less restrictive? I see few options: 1) Allow to save such values and print warning message like "The saved timestamp is before the last saved entry - the search functions may work incorrectly" 2) Add configuration option (#define) to disable timestamp check. This option should be well commented, with information that enabling this option can break up searching functionality if newly saved item timestamp is lesser than previous, use it at your own risk.

armink commented 1 year ago

If you know it's wrong, why do you do it?

valkuc commented 1 year ago

Well, this will work if it's necessary just to display last 100 saved records. Also this will prevent situations like I described in my first message - when the date was changed to the future and then changed back. Because there is no functionality to delete specific TSDB record it can lead to a deadlock - you will not be able to save anything until you either clean whole partition or wait until your actual date will be greater than last saved.

FragrantRye commented 2 months ago

I also support this idea.

I encountered this situation in practice: the device does not have any RTC hardware, so the system timestamp will increase from 0 every time it is powered on. I only hope to save several(like 16KB) log messages in the past, and I don’t care what the real timestamp is.

Can we add an option such as #define TS_OUT_OF_ORDER_SUPPORT to:

  1. Enable out-of-order timestamp insertion.
  2. Downgrade the binary search algorithm to a trivial sequential search?
armink commented 2 months ago

I also support this idea.

I encountered this situation in practice: the device does not have any RTC hardware, so the system timestamp will increase from 0 every time it is powered on. I only hope to save several(like 16KB) log messages in the past, and I don’t care what the real timestamp is.

Can we add an option such as #define TS_OUT_OF_ORDER_SUPPORT to:

  1. Enable out-of-order timestamp insertion.
  2. Downgrade the binary search algorithm to a trivial sequential search?

You can change the fdb_get_time cb impl count++. And restore the count by FDB_TSDB_CTRL_GET_LAST_TIME cmd with fdb_tsdb_control API when system boot.