openhab-addons
openhab-addons copied to clipboard
openhab
[inmemory] Default persistence strategy Forecast
The in memory persistence is especially relevant for forecasts or future value calculations.
To make it easy to configure, I propose a default strategy of Forecast. Any item with time series data will then get persisted in memory. Memory impact is expected to be limited, as typically there are few items with time series. The most likely ones are for weather forecasts and solar predication calculated values.
See discussion: https://community.openhab.org/t/solar-forecast-pv/137681/157?u=mherwege
This would also make it possible to add it as a proposed persistence extension to the setup wizard, without further configuration required. See https://github.com/openhab/openhab-webui/pull/2431
This pull request has been mentioned on openHAB Community. There might be relevant details there:
https://community.openhab.org/t/solar-forecast-pv/137681/161
I don't think this is a good idea. IMO ALL default persistence should be removed, because it does cause a lot of confusion, e.g. RRD files (or tables in the database) for all items are created when the configuruation is loaded later than the first events occur.
Introducing even more complexity here will make it more difficult instead of easier. E.g. having two services providing forecasted values (because inmemory does it by default and the user configures influxdb for the same item) will cause unpredictable results.
I don't think this is a good idea. IMO ALL default persistence should be removed, because it does cause a lot of confusion, e.g. RRD files (or tables in the database) for all items are created when the configuruation is loaded later than the first events occur.
I understand the issue, but this sounds like a different issue to me: persistence add-on loaded and active, but persistence configuration not loaded yet. While removing defaults will solve it, it also forces everyone to create a configuration. And we try to make things simpler for users, not more complex.
Introducing even more complexity here will make it more difficult instead of easier. E.g. having two services providing forecasted values (because inmemory does it by default and the user configures influxdb for the same item) will cause unpredictable results.
What makes things unpredictable? If you don't specify the service when querying, it will use the default service. When you do, it will use the specified one. If you change the default service to influxdb, it will query influxdb by default. Persisting is done to all configured services.
Instead of having the default behaviour hard-coded into the add-on, I would prefer to have the setup wizard create the „default“ config via REST, so the config is a) visible to the user and b) changeable by the user.
Exactly. I would prefer a "persistence assistant" which allows to select an add-on and apply a default configuration for that. I think the current solution is only used because the persistence condifuration is not provided by ConfigurationAdmin and the service can be activated without it. I'll check if we can improve that.
https://github.com/openhab/openhab-core/pull/4137 would expose the default strategy to the UI. Any change would create a real persistence configuration.
Think about initial setup. There are no things and items just yet. But we propose to install addons. Having to come back and configure a persistence configuration when the model (things and items) is created is an extra step that should be optional. There should be reasonable defaults that make it work, but that can be changed.
I am not convinced about removing the concept of default strategy. If we do, I think we then still need some recommended strategy, shown in the UI, that would become the setting after acceptance in the UI. E.g. rrd4j needs every minute to represent things properly. I would keep the default strategy field, but not apply it anymore be default, just use it as a suggestion in the UI. To keep behaviour from previous installations, there is also a need for an upgrade tool to create persistence configurations when there is none and put it to the default. Where do we put it? In text config, or as a managed config? It will create confusion on people wanting text config vs managed config.
I agree with @J-N-K and I would also like to remove the default behaviour from RRD4J, but unfortunately this would be a breaking change. IMO the setup wizard should create persistence configurations for the suggested persistence add-ons (of course only for those getting actually installed), this would be one API request per persistence add-on and a. would make the whole Thing easier to implement backend-wise and b. easier to change the default setup proposed by the setup wizard, because everything is in the UI then.
@J-N-K @florian-h05
As there is no agreement on this, I will close this.
Note there is also a default behaviour for mapDB (restore on startup) which makes a lot of sense.
I am OK if we would remove default behaviour, but we probably need some kind of proposed behaviour when setting up persistence in that case. In my view, the best place to get the proposed behaviour from, is the persistence add-on. What if we repurposed the default behaviour to be proposed behaviour? I don't like making the UI responsible to propose some default behaviour for all current and future persistence services. It is feasible in a setup wizard where one would restrict the presented list, but some kind of proposed behaviour should be there for other services as well. From a backward compatibility perspective, if it is configured through the UI (or there is no text file configuration), we could automatically move the default strategy in the configuration when upgrading (using functionality from https://github.com/openhab/openhab-core/pull/4137). It would be a braking change for users using text configuration.
I am OK if we would remove default behaviour, but we probably need some kind of proposed behaviour when setting up persistence in that case. In my view, the best place to get the proposed behaviour from, is the persistence add-on. What if we repurposed the default behaviour to be proposed behaviour? I don't like making the UI responsible to propose some default behaviour for all current and future persistence services. It is feasible in a setup wizard where one would restrict the presented list, but some kind of proposed behaviour should be there for other services as well.
I have though a bit more of this topic, and it is really difficult.
The default strategy was IMO confusing because it was not transparent to the user, but with https://github.com/openhab/openhab-core/pull/4137 this is different now and very transparent to the user. You have got a point that maintaining default configs for different services inside the UI is also awkward and from an architecture POV not really great. And as moving away from default persistence configs would cause breaking changes that would also have to be dealt with, I guess keeping default configs currently is the best option (before https://github.com/openhab/openhab-core/pull/4137 I had a different opinion about that).
You have got a point that maintaining default configs for different services inside the UI is also awkward and from an architecture POV not really great. And as moving away from default persistence configs would cause breaking changes that would also have to be dealt with, I guess keeping default configs currently is the best option (before openhab/openhab-core#4137 I had a different opinion about that).
@florian-h05 I am happy to see some convergence in views on this. I would still be OK to make 'default persistence' rather something like a 'proposed persistence' that is not automatically applied, but proposed when configuring persistence in the UI, and therefore easily saved as such. But indeed it would not solve the breaking change issue in itself and would require some migration logic for already used persistence without any strategy configured (the default should be copied in). It would also mean all text based configuration without a strategy defined would have to be manually adjusted. And as most persistence configuration is probably still in text files, it would probably have a major impact in the user base.
With this, would that also mean you would support a default forecast persistence strategy for the in memory persistence service? That could then be added as a proposed service in the setup wizard. Everything that is proposed there so far does not require extra configuration for it to work in a reasonable way. With this default strategy, that would be the case as well. I see specific use for this when configuring functionality such as solar or energy price forecasts.
With this, would that also mean you would support a default forecast persistence strategy for the in memory persistence service? That could then be added as a proposed service in the setup wizard.
Yes, I support forecast as the default strategy :+1:
@J-N-K I know you where oposed to this. But could you reconsider based on https://github.com/openhab/openhab-addons/pull/16496#issuecomment-2085155931 and https://github.com/openhab/openhab-addons/pull/16496#issuecomment-2086717039?
If so, I will also create a webui PR to include inmemory persistence in the setup wizard.
@openhab/add-ons-maintainers and @openhab/core-maintainers : what are we deciding here ?
I still think that there should be no default persistence strategies for any persistence add-on. But I won't veto or block this in any other way.
@kaikreuzer @wborn : can we please have your thoughts ?
As we have already 2 default strategies, I am myself not against adding a third one, considering the goal is to simplify the life of users. But that is also true that we have a bug with these default strategies that are sometimes applied while they should not at OH startup. But for the inmemory persistence, is the memory freed in case a more restricted configuration (less items) is applied after the default ? If that is the case, we don't have the same problem for this persistence (while it is a problem for other persistence services as it leads to creation of unwanted files or tables in DB).
I see the value of @mherwege's suggestion to add this as a default strategy. As @lolodomo says, there currently is this concept and it is used in other persistence services, so imho it is ok to have it implemented here as well. The discussion on whether or not we want to keep default strategies is a different one - I'm open to changing this, but as you have already discussed in detail, the tricky parts will be the migration and the separation of concerns (how to keep the logic on how to best persist states for a given service within the service itself).
As @lolodomo says, there currently is this concept and it is used in other persistence services, so imho it is ok to have it implemented here as well.
As there is a large majority in favour of this change, let's merge it.
The discussion on whether or not we want to keep default strategies is a different one
And it can still be debated to find a solution satisfying everyone.
FYI I have just creates https://github.com/openhab/openhab-webui/pull/2629 to add InMemory to the recommended add-ons for the setup wizard.