-
Notifications
You must be signed in to change notification settings - Fork 6
Remove epoch assumption #55
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move uint64_t/pointers higher up in the struct, to improve alignment on 64 bit machines.
Until now we have assumed that clock will always either be correct on boot or be set to epoch. This assumption no longer holds, some of the devices we use have an RTC that seems very hard to disabled. Instead of spending time disabling the RTC, we can instead remove the assumption and this commit is the first step. We store events in the database also before we have a valid timestamp, and then update the timestamp once we have a valid fix. Since we assume that system time starts from zero (this code is not run if we have assumed that timestamp is already valid), what is stored in the database is offset (in time) of the event from the system boot time. When we receive a valid fix, we then calculate the real boot time (time_now - uptime) and then add the real boot time to the offsets we already have. When we remove the assumption about zero or valid, the computation becomes a bit more complicated. The values stored in the database before we have a valid fix is no longer necessarily an offset from zero, but instead a valid timestamp. We therefore need to calculate the boot time relative to system clock when data exporter starts. When we get a valid timestamp, we need to find the difference between the previously mentioned real boot time and the original boot time. This is the offset that will be added to all the timestamps in the database (if any). We still have one assumption, and this that the inital time at boot will always be less than the correct time and that one second is one second. We have some devices where time seems random (hello 2084, 2102 and 2076), but those devices we can force to epoch. If we get a device with forcing fails, we need to make some more adjustmenets to this code. However, I consider that pre-mature optimization and having a very negative outlook on thins (and it is Friday afternoon ...).
Second step of removing epoch assumption. Since timestamps are no longer guaranteed to be offset from 0, we need to remove the original boot time and then add the new boot time. I.e., the first calculation will give us offset from boot time, and the second calculation will give us the correct time of the event.
Replace the "good" timestamp we have used as a limit before with a file that will be written when NTP fix has been obtained. We assume that all devices run NTP (all of our does), and a file seems like the easiest "detect fix" solution that works across devices.
Add debug calculation for uptime.
Use correct type for uint64.
We should read time after we have read uptime. The reason is that if we for example read time at sec.999, then second will have wrapped when we read uptime. While we might risk that the same happens with the new order of operation, it is easier to work with a larger timestamp than a large uptime. For example, with a larger timestamp, we risk having one second as offset to the original boot time. With a small timestamp, we risk having -1 second as offset and that is slightly harder to deal with. The boot time is meant as an estimation anyway, so 100% accurate value is not needed.
Remove some debug out, as the last commit of removing the epoch assumption. There is no need to worry about the time. The issue I tried to solve, was that for example the real boot time could be one second less than the original boot time. This can happen if for example the uptime and timeofday is read at different seconds. One second is not important for us, there is a higher latency in when we discover that a network is down for example. Our goal with the DB calculation is to shift the time offset to that of the boot time. This is done by doing (timestamp - orig_boot_time) + real_boot_time, for all timestamps less than real_boot_time. If real_boot_time is less than orig_boot_time, then the statement has no effect. This is ok, as we input absolute time in the database. The code is meant to handle large differences in time, for example from epoch or some default rtc value. And that works fine.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit removes the assumption that time always starts from epoch from the sqlite writer. Instead, we try to estimate the real boot time and original boot time. When we know that we have an ntp fix (currently designed to be signalled by the presence of a file), we update the timestamps in the database for timestamps less than the real boot time. The new timestamp is set to (timestamp - origianl boot time) + real_boot_time. Thus, we get the offset from the original boot time and set the timestamp to offset from the real boot time.
When the time is correct and data exporter starts, we can get into a situation where real boot time is less than original boot time. This will turn the SQL query into a nop, but that is not a problem. The code is meant to handle large jumps in time (like from epoch) and we insert absolute times into the database, so an error of a second is not a problem.