Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
valkuc opened this issue Nov 7, 2023 · 4 comments

Comments

@valkuc
Copy link
Contributor

valkuc commented Nov 7, 2023

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
Copy link
Owner

armink commented Nov 9, 2023

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

@valkuc
Copy link
Contributor Author

valkuc commented Nov 11, 2023

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
Copy link
Owner

armink commented Nov 12, 2023

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

@valkuc
Copy link
Contributor Author

valkuc commented Nov 12, 2023

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.

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

No branches or pull requests

2 participants