Add last_changed_api_heartbeat_state_on__date to the device model #1343

thgreasi · 2023-05-09T11:09:23Z

Resolves: #644
Change-type: minor

thgreasi · 2023-05-09T13:49:10Z

src/features/device-heartbeat/index.ts

-				body,
+				body: {
+					...baseBody,
+					last_api_heartbeat_state_change_event: Date.now(),


I originally added this as a hook, but at the end I preferred moving it here

so that we can avoid the extra .get() request to confirm that the value has indeed changed, or blindly setting last_api_heartbeat_state_change_event whenever values.api_heartbeat_state != null.

since this is the only place that we should be updating the heartbeat anyway.

Let me know though if you prefer to switch it back to hooks.

fisehara

Unfortunately I don't understand properly what this added property adds to the device state information?
Do we want to convey the last proper heartbeat time or do we want to surface all changes of heartbeat event states?
Eg. when it's set to online over and over again, it would be of interest when the last time the heartbeat was set to online, or am I mixing things up?

fisehara · 2023-05-17T07:22:22Z

src/features/device-heartbeat/index.ts

+				body: {
+					...baseBody,
+					last_changed_api_heartbeat_state_on__date: Date.now(),
+				},


I wonder if last_changed_api_heartbeat_state_on__date should only be updated when called via: await this.updateDeviceModel(deviceId, DeviceOnlineStates.Online);
and not when patching it to timeout or offline? As these events are internal events that do not show the device are heartbeating anymore?

The intention here is to track when the heartbeat was changes both for any reason (both from state-GETs and from heartbeat changes b/c of the heartbeat mechanism). Knowing then the device switched to Timeout/Offline is probably more useful than knowing since when the device originally started heartbeating.

fisehara · 2023-05-17T07:27:30Z

test/03_device-state.ts

+								await expectResourceToMatch(
+									getActor(),
+									'device',
+									getDevice().id,
+									{
+										api_heartbeat_state: DeviceOnlineStates.Timeout,
+										last_changed_api_heartbeat_state_on__date:
+											thatIsDateStringAfter(stateUpdatedAfter),
+									},


When we use the last_changed_api_heartbeat_state_on__date what do we want to convey? Do we want to convey when the device was last seen with a proper heartbeat or when the heartbeat status changes from online, to timeout, to offline?
How will we use the timeout and offline timestamps, eg as indicator in the UI?

We want to convey when the heartbeat status changed in general for all possible transitions.
Similarly to how UI atm uses the last vpn even to show

online since

offline since

with this field it can show

heartbeating since

heartbeat timeout since

heartbeat offline since

On top of that this also allows us t compute the last heartbeat if needed on read time:

Given this field, with a simple computation using the device's poll interval, a client/computed term could easily compute even more useful information on read time, eg: compute the last heartbeat for a non-Online device.

thgreasi · 2023-05-17T08:31:35Z

Unfortunately I don't understand properly what this added property adds to the device state information?
Do we want to convey the last proper heartbeat time or do we want to surface all changes of heartbeat event states?
Eg. when it's set to online over and over again, it would be of interest when the last time the heartbeat was set to online, or am I mixing things up?

@fisehara in an ideal world we would add a device has last heartbeat on date, but that would tons of load on the writer, effectively making all state GETs do DB writes. This is exactly why we had to add the heartbeat update cache mechanism in the heartbeat implementation, so that we can reduce the writer load.

Given that we can't do the above, we started thinking what fact related to heartbeat we could store that would come with low DB load impact, and that's how we ended up with last_changed_api_heartbeat_state_on__date in that arch call since:

it doesn't add extra DB load, since we do the write along with the operation that changes the heartbeat (and PG rewrites the whole row anyway)
we can compute more interesting information on read based on this

Part of the reasoning in the arch call was that we atm have no additional info about the heartbeat, so adding any kind of free field is better than the no info that we atm have.

Even w/o extra computations, being able to surface when a device last switched to Online/Timeout/Offline does have some value, especially when compared w/ how atm we have no more info about the heartbeat (and we have to dive into Redis to get more).

Given this field, with a simple computation using the device's poll interval, a client/computed term could easily compute even more useful information on read time, eg: compute the last heartbeat for a non-Online device.

Resolves: #644 Change-type: minor

thgreasi · 2024-03-20T13:02:49Z

Thank you @fisehara I will make this part of the next cycle, so that we confirm the name again ❤️

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch 2 times, most recently from 61eb4da to 8796748 Compare May 9, 2023 13:17

thgreasi requested a review from Page- May 9, 2023 13:44

thgreasi commented May 9, 2023

View reviewed changes

thgreasi marked this pull request as ready for review May 9, 2023 13:49

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch from 8796748 to f9d82ef Compare May 16, 2023 14:52

flowzone-app bot enabled auto-merge May 16, 2023 14:53

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch from f9d82ef to 2475c4f Compare May 16, 2023 14:54

thgreasi changed the title ~~Add last_api_heartbeat_state_change_event to the device model~~ Add last_changed_api_heartbeat_state_on__date to the device model May 16, 2023

thgreasi requested a review from fisehara May 16, 2023 14:55

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch from 2475c4f to 1f0fb21 Compare May 16, 2023 14:55

fisehara reviewed May 17, 2023

View reviewed changes

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch 2 times, most recently from cb7b927 to 3cf1097 Compare June 14, 2023 19:46

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch from 3cf1097 to 6adb950 Compare July 17, 2023 09:19

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch from 6adb950 to 31ff1bd Compare August 30, 2023 06:58

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch from 31ff1bd to f4251ac Compare September 28, 2023 07:13

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch from f4251ac to bcc5701 Compare October 30, 2023 07:15

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch 3 times, most recently from 72f0293 to e1a0271 Compare December 15, 2023 15:50

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch from e1a0271 to 934e78a Compare February 6, 2024 15:57

Add last_changed_api_heartbeat_state_on__date to the device model

e604777

Resolves: #644 Change-type: minor

thgreasi force-pushed the 644-last_api_heartbeat_state_change_event branch from 934e78a to e604777 Compare March 13, 2024 16:45

fisehara approved these changes Mar 20, 2024

View reviewed changes

thgreasi marked this pull request as draft April 3, 2024 12:56

auto-merge was automatically disabled April 3, 2024 12:56
Pull request was converted to draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add last_changed_api_heartbeat_state_on__date to the device model #1343

Add last_changed_api_heartbeat_state_on__date to the device model #1343

thgreasi commented May 9, 2023 •

edited

thgreasi May 9, 2023

fisehara left a comment

fisehara May 17, 2023

thgreasi May 17, 2023

fisehara May 17, 2023

thgreasi May 17, 2023

thgreasi commented May 17, 2023 •

edited

thgreasi commented Mar 20, 2024

Add last_changed_api_heartbeat_state_on__date to the device model #1343

Are you sure you want to change the base?

Add last_changed_api_heartbeat_state_on__date to the device model #1343

Conversation

thgreasi commented May 9, 2023 • edited

thgreasi May 9, 2023

Choose a reason for hiding this comment

fisehara left a comment

Choose a reason for hiding this comment

fisehara May 17, 2023

Choose a reason for hiding this comment

thgreasi May 17, 2023

Choose a reason for hiding this comment

fisehara May 17, 2023

Choose a reason for hiding this comment

thgreasi May 17, 2023

Choose a reason for hiding this comment

thgreasi commented May 17, 2023 • edited

thgreasi commented Mar 20, 2024

thgreasi commented May 9, 2023 •

edited

thgreasi commented May 17, 2023 •

edited