-
-
Notifications
You must be signed in to change notification settings - Fork 428
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
Persistence restore lastState and lastStateChange on startup #4463
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Mark Herwege <[email protected]>
Signed-off-by: Mark Herwege <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the persistence support for this!
historicItem.getState()); | ||
} | ||
GenericItem genericItem = (GenericItem) item; | ||
if (!UnDefType.NULL.equals(item.getState())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this not done before querying the persistence service?
It seems that we could've avoided running the query unnecessarily if the state was already initialised.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess, as the query runs in a separate thread, the item state may have changed in between starting the query and returning the result.
@J-N-K introduced this. I didn't find more explanation for it, but this was my interpretation. So I think it should remain there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess, as the query runs in a separate thread, the item state may have changed in between starting the query and returning the result.
That makes sense, but I think it would also make sense to check it beforehand as well.
But perhaps if that were the case, such change could be added later, and separately from this PR.
This PR requires #4351
It extends restoreOnStartup to also restore the
lastState
,lastStateUpdate
andlastStateChange
GenericItem fields. This allows having these fields available and set immediately after a startup without needing a state change.@jimtng FYI