ptrdom opened a new issue, #181: URL: https://github.com/apache/pekko-persistence-r2dbc/issues/181
Follow up on https://github.com/apache/pekko/issues/1513. Implementing persistence plugin configuration at runtime for this project will require changes to implementation and configuration. Because the required changes are not trivial and there potentially are multiple ways to approach them, I am going to give more context and propose what I think is the simplest solution. ## Current behavior Configuration for persistence plugins is provided roughly as follows: ``` pekko.persistence.r2dbc { journal { class = "org.apache.pekko.persistence.r2dbc.journal.R2dbcJournal" # other settings } } pekko.persistence.r2dbc { snapshot { class = "org.apache.pekko.persistence.r2dbc.snapshot.R2dbcSnapshotStore" # other settings } } pekko.persistence.r2dbc { state { class = "org.apache.pekko.persistence.r2dbc.state.R2dbcDurableStateStoreProvider" # other settings } } pekko.persistence.r2dbc { query { class = "org.apache.pekko.persistence.r2dbc.query.R2dbcReadJournalProvider" # other settings } } pekko.persistence.r2dbc { dialect = “postgres” connection-factory { host = "localhost" port = 5432 database = "postgres" user = "postgres" password = "postgres" } # other settings } ``` The way this config is used is that journal, query, snapshot and state plugins take this whole config from the `ActorSystem`, pick their specific configuration from one of the branches - `pekko.persistence.r2dbc.journal`, `pekko.persistence.r2dbc.query`, etc. - and then pick the shared configuration - like database connection settings - from parent path of `pekko.persistence.r2dbc`. Additionally, journal plugin is the only plugin that directly depends on another plugin - it uses the query plugin for replay of events. ## Problems with current behavior There are two problems with this implementation that makes it incompatible with how runtime plugin configuration works: 1) Persistence plugins do not use config that is passed to them, instead taking the config from the `ActorSystem`. https://github.com/apache/pekko-persistence-r2dbc/blob/553752fc82c15695e52c8db7627a48b1ec14bd3c/core/src/main/scala/org/apache/pekko/persistence/r2dbc/journal/R2dbcJournal.scala#L97 Constructor argument `config` is not used in current implementation. This can be observed in implementations of all plugins. 1) Journal plugin does not pass dynamic configuration to the query plugin that it instantiates. https://github.com/apache/pekko-persistence-r2dbc/blob/553752fc82c15695e52c8db7627a48b1ec14bd3c/core/src/main/scala/org/apache/pekko/persistence/r2dbc/journal/R2dbcJournal.scala#L100 `PersistenceQuery` has a separate `readJournalFor` method that accepts configuration as the second parameter and this method is not used here. ## Expected behavior Expectation is simply that persistence plugins should actually use the config that is provided to them via constructor instead of using the config from the `ActorSystem`. ## Proposed solution 1) Use configuration from `config` constructor argument in persistence plugins. There are few changes required to make this work. Configuration that is passed through this argument has certain constraints, for example it has to have a `class` attribute equals to the FQCN of the class implementing chosen plugin. Essentially it must be the plugin specific branch of the config - `pekko.persistence.r2dbc.journal`, `pekko.persistence.r2dbc.query`, etc., which means that shared configuration can no longer be defined at parent path level, because this will not get passed with `config` argument. So the solution would be just that - to move in the shared config into each of the plugin branches: ``` pekko.persistence.r2dbc { journal { class = "org.apache.pekko.persistence.r2dbc.journal.R2dbcJournal" shared = ${pekko.persistence.r2dbc.shared} # other settings } } pekko.persistence.r2dbc { snapshot { class = "org.apache.pekko.persistence.r2dbc.snapshot.R2dbcSnapshotStore" shared = ${pekko.persistence.r2dbc.shared} # other settings } } pekko.persistence.r2dbc { state { class = "org.apache.pekko.persistence.r2dbc.state.R2dbcDurableStateStoreProvider" shared = ${pekko.persistence.r2dbc.shared} # other settings } } pekko.persistence.r2dbc { query { class = "org.apache.pekko.persistence.r2dbc.query.R2dbcReadJournalProvider" shared = ${pekko.persistence.r2dbc.shared} # other settings } } pekko.persistence.r2dbc.shared { dialect = “postgres” connection-factory { host = "localhost" port = 5432 database = "postgres" user = "postgres" password = "postgres" } # other settings } ``` This change will also require split of `R2dbcSettings`: https://github.com/apache/pekko-persistence-r2dbc/blob/553752fc82c15695e52c8db7627a48b1ec14bd3c/core/src/main/scala/org/apache/pekko/persistence/r2dbc/R2dbcSettings.scala#L40-L75 Because one plugin branch will not contain other plugin branches, this class will need to be split to a class per plugin, potentially with `shared` config expressed through an attribute of a common type: ```scala class R2dbcJournalSettings( journalTable: String, shared: R2dbcSharedSettings // journal specific settings ) class R2dbcJournalSettings( snapshotsTable: String, shared: R2dbcSharedSettings // other snapshot specific settings ) // other classes for other plugins class R2dbcSharedSettings( dialect: Dialect, schema: String // other shared settings ) ``` 1) Journal plugin should not depend on query plugin. This will simplify the config for journal plugin, because it will not have to dynamically initialize query plugin. The functionality that is common between journal and query plugins - fetching of events by persistence ID - can just be moved to a common trait/class that both plugins depend on. With this solution implemented the runtime plugin configuration would be used as follows: ```scala // provide the whole config for required plugins val config = ConfigFactory .parseString(""" runtime1 { journal { class = "org.apache.pekko.persistence.r2dbc.journal.R2dbcJournal" shared = ${runtime1.shared} # other settings } snapshot { class = "org.apache.pekko.persistence.r2dbc.snapshot.R2dbcSnapshotStore" shared = ${runtime1.shared} # other settings } shared = { dialect = "postgres" connection-factory { database = "runtime1_database" # other settings } } } """) .resolve() // or do overrides for defaults val config = ConfigFactory .parseString(""" runtime1 { journal = ${pekko.persistence.r2dbc.journal} journal.shared = ${runtime1.shared} snapshot = ${pekko.persistence.r2dbc.snapshot} snapshot.shared = ${runtime1.shared} shared = ${pekko.persistence.r2dbc.shared} shared = { dialect = "postgres" connection-factory { database = "runtime1_database" } } } """) .withFallback(ConfigFactory.load()) .resolve() EventSourcedBehavion... .withJournalPluginId("runtime1.journal") .withJournalPluginConfig(config) .withSnapshotPluginId("runtime1.snapshot") .withSnapshotPluginConfig(config) ``` Please share your thoughts on the proposed solution. Is it good to go as is or are there any aspects that need improvement? The only problem I see right now is that both binary and source compatibility look like a major pain to preserve. @pjfanning, pinging you since you have participated in this development. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org For additional commands, e-mail: notifications-h...@pekko.apache.org
