search

akka / akka-persistence-jdbc

Asynchronously writes journal and snapshot entries to configured JDBC databases so that Akka Actors can recover state
https://doc.akka.io/docs/akka-persistence-jdbc/
Other
308 stars 142 forks source link

Change the default for akka-persistence-jdbc.logicalDeletion.enable #557

Open patriknw opened 3 years ago

patriknw commented 3 years ago

Soft (logical) deletes is still the default. I can imagine that is surprising for many. Logical deletes has been deprecated and there is a warning logged.

ignasi35 commented 3 years ago

I haven't jumped into this codebase in a while and realized that logicalDelete is only used in the (now legacy) ByteArrayJournalDao. The ByteArrayJournalDao class is not deprecated but was moved to the akka.persistence.jdbc.journal.dao.legacy package prior to version 5.0.0. See https://github.com/akka/akka-persistence-jdbc/pull/467

If I am reading the code and history correctly, in v5.0.0 the new DefaultJournalDao was introduced and that new DAO doesn't support logicalDelete.

Then, if introducing a change like switching the default value of akka-persistence-jdbc.logicalDeletion.enable from false to true already requires a MAJOR version bump, I wonder if we could, instead, drop/deprecate the ByteArrayJournalDao and require users to migrate their data and use DefaultJournalDao.

octonato commented 3 years ago

require users to migrate their data

That's the problem. We don't have a migration tool yet. That's why we're keep the legacy DAO for now.

The is a migration tool in the making (#501). I was even planning to pick it this week, but didn't find the time.

ignasi35 commented 3 years ago

Sure, what I'm saying is that in a (hypothetical) version v6.0.0 of this plugin we would have the migration tool (from the legacy ByteArrayJournalDao to the DefaultJournalDao). Then, changing the default value of logicalDeletion.enable is no longer valuable. Users gain a lot more from migrating than from adopting the new default value of logicalDeletion.enable and staying on ByteArrayJournalDao. I would just deprecate ByteArrayJournalDao in v5.0.x already or add a log trace linking to an issue explaining the plans to remove it when the migration tool is completed.

IMHO; any improvement on ByteArrayJournalDao is little to no value.