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

Introduce state entry timestamping and use it to fix apcupsd-ups #2010

Merged
merged 21 commits into from
Aug 5, 2023

Conversation

jimklimov
Copy link
Member

@jimklimov jimklimov commented Aug 3, 2023

CC @aquette @clepple : This is a fairly noticeable change to the deep internals, state info trees are at the core of everything in NUT. Would welcome another set of eyeballs that I got this conceptually right :)

Should I be concerned about computational overheads to keep reading and comparing time values? Should this behavior become a toggle (e.g. for programs that know they intend to use this feature at all)?

In context of issue #2007 there is a need to somehow avoid the current behavior with deletion of nearly all info about the device and then re-querying apcupsd to populate the dstate again. With this change we hopefully (un-tested so far) have a way to determine if any entries were not updated in the last read, and so should be cleaned away AFTER talking to the device (apcupsd server in this case).

While this is a first use for such tech, I can see it proliferating into other drivers to support "Data stale" decisions (e.g. attempt to re-initialize the hardware connection), etc.

Closes: #2007
Fixes: #797 fallout

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…pstools#797 fallout]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…meval()

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…eteness

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…te_get_timestamp() and st_tree_node_compare_timestamp() to track age of state entries

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…oleted readings AFTER refreshing data from apcupsd daemon [networkupstools#2007]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…FTER refreshing data from apcupsd daemon [networkupstools#2007]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
… a new value

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@clepple
Copy link
Member

clepple commented Aug 4, 2023

Benchmarking by eyeball is always dubious at best. But given that the values going into the state info trees are converted to C strings, I have a feeling that the timings for int-to-double conversion of the timestamps will be dwarfed by the conversion to strings (at least on modern platforms).

I definitely don't claim to be an expert on the state trees, so I wouldn't trust myself to notice whether there are subtle bugs being introduced.

… of got_monoclock in upsnotify() [networkupstools#1777 follow-up]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…etworkupstools#1777 follow-up]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…2010]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…ollow-up]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…x expression [networkupstools#2007]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
… we are deleting an entry because it is too old [networkupstools#2007]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
@jimklimov
Copy link
Member Author

Checked locally, does not regress with an Eaton UPS (not impacted directly like apcupsd-ups which was deliberately changed by this PR, but impacted indirectly by change of state/dstate tech). Seems to work still quickly and report same data :)

@jimklimov
Copy link
Member Author

Also checked in original issue #2007 to actually fix the problem the user came with :)

@jimklimov jimklimov merged commit 3a11558 into networkupstools:master Aug 5, 2023
47 of 48 checks passed
@jimklimov jimklimov deleted the issue-2007 branch August 5, 2023 15:07
alexwbaule pushed a commit to alexwbaule/nut that referenced this pull request Oct 4, 2023
…2010]

Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

apcupsd-ups driver race condition
2 participants