apache / pekko-persistence-r2dbc

Asynchronously writes journal and snapshot entries to configured R2DBC databases so that Apache Pekko Actors can recover state
https://pekko.apache.org/
Apache License 2.0
14 stars 6 forks source link

Revisit usage of `pekko_` schema prefix? #19

Closed mdedetrich closed 9 months ago

mdedetrich commented 1 year ago

One thing I noticed that this plugin using the pekko_ (formerly akka_) prefix for schema is in contrast to https://github.com/apache/incubator-pekko-persistence-jdbc which doesn't have any such schema. Maybe we should have a look into removing the prefix entirely and as an added bonus this might make this plugin compatible with https://github.com/apache/incubator-pekko-persistence-jdbc since the schema would be the same (giving people an easy upgrade path from JDBC to r2dbc without any migration).

Related to https://github.com/apache/incubator-pekko-persistence-r2dbc/issues/17

mdedetrich commented 1 year ago

@nvollmar @spangaer @danischroeter I think you use persistence in production so maybe you can comment on this? (Also feel free to ping anyone else that may also be using it)

danischroeter commented 1 year ago

@mdedetrich I think the prefix should be removed. It does not add value and is inconsistent. Persistence-cassandra also does not have a prefix. E.g journal table just called messages see https://github.com/apache/incubator-pekko-persistence-cassandra/blob/main/core/src/main/scala/org/apache/pekko/persistence/cassandra/journal/CassandraJournalStatements.scala https://github.com/apache/incubator-pekko-persistence-cassandra/blob/main/core/src/main/resources/reference.conf#L105

More interestingly most tables do not have a prefix - just projection

see https://github.com/apache/incubator-pekko-persistence-r2dbc/blob/main/ddl-scripts/create_tables_postgres.sql

DROP TABLE IF EXISTS event_journal;
DROP TABLE IF EXISTS snapshot;
DROP TABLE IF EXISTS durable_state;
DROP TABLE IF EXISTS pekko_projection_offset_store;
DROP TABLE IF EXISTS pekko_projection_timestamp_offset_store;
DROP TABLE IF EXISTS pekko_projection_management;
mdedetrich commented 1 year ago

In that case I would say there is merit in doing this as well as making the schema consistent with https://github.com/apache/incubator-pekko-persistence-jdbc since they both deal with SQL/SQL like databases. Since we have to do a migration anyways I think there is an argument that it can be released with v1.0.0 if someone implements this before the release is made (that way we don't need another migration for v1.1.x)

pjfanning commented 10 months ago

@mdedetrich what do you think we should do here? We're close enough to being able to consider an RC for this module.

mdedetrich commented 9 months ago

@danischroeter Are you interested in looking into this? We are contemplating doing a release of persistence-r2dbc soon and being a schema change it makes sense to fix this before an initial release.

danischroeter commented 9 months ago

@mdedetrich Sure. I'll look into it.

danischroeter commented 9 months ago

@mdedetrich I just checked if we could make this plugin compatible with https://github.com/apache/incubator-pekko-persistence-jdbc This is not possible. The schemas although similar are fundamentally different - especially related to tags.

jdbc stores tags in a separate table event_tag vs r2dbc uses tags TEXT ARRAY in the event_journal table.

The two plugins are essentially incompatible and must use different schemas. One can use the migration tool to convert from jdbc to r2dbc. See https://pekko.apache.org/docs/pekko-persistence-r2dbc/current/migration.html

I think I will add a note to the schema documentation to make this incompatibility obvious.