Configuring persistence plugins at runtime

[ptrdom]

Resolves #181.

TODO:

• Break up pekko.persistence.rdbc.shared config.
  Shared config is now bloated - plugins have access to config keys they are not using. Ideally shared configuration should be broken into multiple feature-specific sub-configs and plugins would only depend on sub-configs that they are using.
• Implement shared connection factories.
  With changes to config and ConnectionFactoryProvider the connection factories are no longer shared between plugins by default and there is no way to make them shared. Need to re-implement this feature, best idea now seems to be to copy pekko-persistence-jdbc implementation.
• Deal with projection module.
  Even though runtime config is available already with R2dbcProjectionSettings, changes to config of core module and ConnectionFactoryProvider do affect projection module.

This PR will become (or already is) rather large, maybe there are sensible ways to break it up. By sensible I mean that the project would be in a releasable state between sub-PRs.

[ptrdom]

While working on the initial design I spotted few overcomplications with it, particularly when it came to implementation of shared connection factories, so I made a few changes to simplify:

1. Added new method to ConnectionFactoryProvider implementation that accepts config.

   You can now both supply full path to conneciton factory config and config in which this path should be looked up. If the supplied config is missing this path, then it will be looked up in ActorSystem's config. What persistence plugins now do is pass the config that is passed to them at runtime to this new method.

   This allows configuration of all expected scenarios - startup-time and runtime config for both shared and non-shared connection factories. Connection factories remain shared by default as they were before. Full path remains the ID of connection factory for sharing purposes.

   This is very similar to how persistence plugin config is already implemented, so should feel familiar to users.

2. Plugin config uses use-connection-factory config key instead of direct plugin configuration.

   This is how projection module already does it on its side, and it actually works really nicely for all use cases - if you want to share the connection factory, then just put it on the same path that all plugins can use, if you want to use unique connection factories, then you just need to put the config on an unique path. With an implementation like this there is no need for special purpose config keys - like the ones that JDBC plugin uses.

With these changes the other TODOs could be done in their own tickets:

1. Shared settings are now provided through inheritance:

   pekko-persistence-r2dbc/core/src/main/resources/reference.conf

   Lines 4 to 8 in b289680

   	pekko.persistence.r2dbc {
   	// TODO temporary solution, change to only using keys that are actually used by the implementation
   	journal = ${pekko.persistence.r2dbc}
   	journal {
   	class = "org.apache.pekko.persistence.r2dbc.journal.R2dbcJournal"

   Ideally the break up of shared settings would still be done, but it does not seem as critical.

2. Projection module remains compatible with core module changes done in this PR, but it is not compatible with runtime persistence plugin config feature.

   Even though use-connection-factory can be set at runtime, projection using it becomes sensitive to initialization order, because projection cannot see runtime config that is provided to the persistence plugin, so if the projection runs before the plugin is initialized, it will fail. I feel that users should not be expected to be careful of that. Also, if persistence plugin and projection are running in separate ActorSystems, then even the tight control of the initialization order will not make the projection work.

   This will be addressed in a separate PR.

I will look though the PR again for any things that were potentially missed, clean it up and mark as ready for review.

[ptrdom]

I think this is ready for review.

Not sure about the license headers on new files - test is a new creation, so a standard header I think, but two new DAO classes are extracted code from pre-Pekko/Akka split class.

Going to fix binary compatibility checks once the code is final.

[ptrdom]

1. Shared settings are now provided through inheritance:
   

   pekko-persistence-r2dbc/core/src/main/resources/reference.conf

   Lines 4 to 8 in b289680

   	pekko.persistence.r2dbc {
   	// TODO temporary solution, change to only using keys that are actually used by the implementation
   	journal = ${pekko.persistence.r2dbc}
   	journal {
   	class = "org.apache.pekko.persistence.r2dbc.journal.R2dbcJournal"

   Ideally the break up of shared settings would still be done, but it does not seem as critical.

Maybe even without the break up of shared settings it would still make sense to put them under pekko.persistence.r2dbc.shared key, which would then be inherited by plugin configs instead of whole pekko.persistence.r2dbc.

[pjfanning]

currently (just for reference)

[error] pekko-persistence-r2dbc: Failed binary compatibility check against org.apache.pekko:pekko-persistence-r2dbc_2.12:1.0.0! Found 2 potential problems
[error]  * class org.apache.pekko.persistence.r2dbc.R2dbcSettings does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("org.apache.pekko.persistence.r2dbc.R2dbcSettings")
[error]  * object org.apache.pekko.persistence.r2dbc.R2dbcSettings does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("org.apache.pekko.persistence.r2dbc.R2dbcSettings$")

[pjfanning]

new files that are not copies of old Akka files should be given the standard Apache header not the Akka one

[pjfanning]

I am not sure that renaming so many config settings is a good idea. Can we at least produce a doc that details all the renames? That is a list of c1 becomes c2 pairs.

[ptrdom]

I am not sure that renaming so many config settings is a good idea. Can we at least produce a doc that details all the renames? That is a list of c1 becomes c2 pairs.

Sure I can make a list.

We do need to decide if we want to make changes a correct implementation that also support runtime plugin config or want to preserve backwards compatibility. For example, current main branch durable state uses buffer-size setting from query settings. I think that is simply incorrect implementation that should not have made it into main branch or at least 1.0.0 release. There are two ways I see to do it better:

1. Move buffer-size under pekko.persistence.r2dbc key, where shared settings are located. Both durable state and query plugin inherit this key and can have their own overrides to it. This is what this PR does.
2. Make query and durable state have their own buffer-size settings, without inheritance from shared settings under pekko.persistence.r2dbc. Shared settings would not have buffer-size, because buffer-size is feature-specific and should have been made so. I think this would be ideal solution, but would bloat this PR with even more config changes.

We cannot make runtime plugin config work without making plugin configuration self-contained, which it is not right now, so we have to move/rename configuration to some extent to achieve self-containment. If there is great focus on keeping backwards compatibility and sticking to keeping configuration as it is now, then we cannot provide runtime plugin config feature in this repository. This feature will have to be a major release feature, or not a feature at all. What are your thoughts on this @pjfanning?

[ptrdom]

Sure I can make a list.

And what would be the purpose of this list? Like would it just be a part of release documentation, or something else? If this is about release documentation, then I think we need to finalize the config changes before documenting them.

[ptrdom]

We do need to decide if we want to make changes a correct implementation that also support runtime plugin config or want to preserve backwards compatibility.

And of course there is a spectrum here rather just former or latter, so we need to decide on where we are on the spectrum. There is likely a way to somehow contort this PR into not even requiring a major release, but since config moves/renames were proposed in the initial issue and were not explicitly rejected, I assumed that it is fine.

[pjfanning]

I don't mind calling the next release 2.0.0. I just want to limit the breaking changes and be able to fully document the changes.