Configuring persistence plugins at runtime for EventSourcedBehavior #1518

ptrdom · 2024-10-06T16:54:06Z

Resolves #1513.

…icitly

pjfanning · 2024-10-06T17:49:23Z

...e-testkit/src/main/scala/org/apache/pekko/persistence/testkit/PersistenceTestKitPlugin.scala

@@ -112,9 +112,12 @@ object PersistenceTestKitPlugin {
 * Persistence testkit plugin for snapshots.
 */
 @InternalApi
-class PersistenceTestKitSnapshotPlugin extends SnapshotStore {
+class PersistenceTestKitSnapshotPlugin(@unused cfg: Config, cfgPath: String) extends SnapshotStore {


why do we need an unused parameter?

It is because of this bit of code:

pekko/persistence/src/main/scala/org/apache/pekko/persistence/Persistence.scala

Lines 413 to 425 in 2421458

val pluginActorArgs: List[AnyRef] =

try {

Reflect.findConstructor(pluginClass, List(pluginConfig, configPath)) // will throw if not found

List(pluginConfig, configPath)

} catch {

case NonFatal(_) =>

try {

Reflect.findConstructor(pluginClass, List(pluginConfig)) // will throw if not found

List(pluginConfig)

} catch {

case NonFatal(_) => Nil

} // otherwise use empty constructor

}

Easiest solution seemed to work around this the same as it is done in PersistenceTestKitPlugin.

so if the param is used, can you removed the @unused annotation?

It is used only through reflection, so compilation throws the following if @unused is removed:

parameter cfg in class PersistenceTestKitSnapshotPlugin is never used [error] Applicable -Wconf / @nowarn filters for this fatal warning: msg=<part of the message>, cat=unused-params, site=org.apache.pekko.persistence.testkit.PersistenceTestKitSnapshotPlugin.cfg [error] class PersistenceTestKitSnapshotPlugin(cfg: Config, cfgPath: String) extends SnapshotStore { [error] ^ [error] one error found [error] (persistence-testkit / Compile / compileIncremental) Compilation failed

fair enough - could you add a code comment explaining the @unused?

...n/scala/org/apache/pekko/persistence/testkit/internal/SnapshotStorageEmulatorExtension.scala

...stkit/src/test/scala/org/apache/pekko/persistence/testkit/scaladsl/RuntimeJournalsSpec.scala

pjfanning

there are quite a lot of changes to constructors - even if they are for internal classes

I don't know but we might want to consider adding secondary constructors that match the existing param lists in the 1.1.2-RC1 release.

...istence-typed/src/main/scala/org/apache/pekko/persistence/typed/internal/BehaviorSetup.scala

pjfanning · 2024-10-06T17:57:09Z

...istence-typed/src/main/scala/org/apache/pekko/persistence/typed/internal/BehaviorSetup.scala

@@ -13,10 +13,12 @@

 package org.apache.pekko.persistence.typed.internal

+import com.typesafe.config.ConfigFactory


keep the scala namespace imports at the start

...istence-typed/src/main/scala/org/apache/pekko/persistence/typed/internal/BehaviorSetup.scala

pjfanning · 2024-10-06T17:57:48Z

...istence-typed/src/main/scala/org/apache/pekko/persistence/typed/internal/BehaviorSetup.scala

@@ -107,14 +113,18 @@ private[pekko] final class BehaviorSetup[C, E, S](

  private var recoveryTimer: OptionVal[Cancellable] = OptionVal.None

+  val recoveryEventTimeout: FiniteDuration = persistence


private please

This is accessed in multiple places of EventSourcedBehavior state machine - ReplayingEvents and ReplayingSnapshot are using this to make error messages, so cannot be made private. Please advise if some different solution is required.

Can you add a @since 1.1.3 annotation to any new public classes, variables or functions?

Just double-checking - is this required for public members of classes marked with @InternalApi and private[pekko]?

I'm not sure - this whole PR is looking like something we might need to stick in a future major release as opposed to an upcoming patch release. Leave the since annotations out for now.

Understood. Is there any issue with creating a PR with this code in Akka repository? Wondering about any potential license conflicts, would not want interaction with BSL repositories voiding Pekko of this code.

Feel free to submit this to Akka. You own this code and submitting it Akka does not change that. The problem will only arise if you collaborate on some of the code with a Lightbend employee and then take that collaboration and try to submit that to us.

pjfanning · 2024-10-07T19:05:22Z

There are binary compat problems that will require adding an exclude file for.

[error] pekko-persistence-typed: Failed binary compatibility check against org.apache.pekko:pekko-persistence-typed_2.12:1.0.0! Found 2 potential problems
[error]  * abstract method withJournalPluginConfig(scala.Option)org.apache.pekko.persistence.typed.scaladsl.EventSourcedBehavior in interface org.apache.pekko.persistence.typed.scaladsl.EventSourcedBehavior is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("org.apache.pekko.persistence.typed.scaladsl.EventSourcedBehavior.withJournalPluginConfig")
[error]  * abstract method withSnapshotPluginConfig(scala.Option)org.apache.pekko.persistence.typed.scaladsl.EventSourcedBehavior in interface org.apache.pekko.persistence.typed.scaladsl.EventSourcedBehavior is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("org.apache.pekko.persistence.typed.scaladsl.EventSourcedBehavior.withSnapshotPluginConfig")
[error] java.lang.RuntimeException: Failed binary compatibility check against org.apache.pekko:pekko-persistence-typed_2.12:1.0.0! Found 2 potential problems

pjfanning · 2024-10-08T19:52:28Z

...e-typed/src/main/scala/org/apache/pekko/persistence/typed/javadsl/EventSourcedBehavior.scala

@@ -16,6 +16,8 @@ package org.apache.pekko.persistence.typed.javadsl
 import java.util.Collections
 import java.util.Optional

+import scala.jdk.OptionConverters._


there is a Pekko OptionConverters - can you use that? org.pekko.util

We can't use scala.jdk.OptionConverters because we still support Scala 2.12

ptrdom added 5 commits October 3, 2024 22:07

Add runtime journal plugin configuration to EventSourcedBehavior

0984a15

Add runtime journal plugin configuration test for event journals

31a3a02

Add runtime journal plugin configuration test for snapshot store

4ef71d2

Mark SnapshotStorageEmulatorExtension class as internal API

c39d403

Add runtime journal plugin configuration test for event journals expl…

fb68e0e

…icitly

pjfanning reviewed Oct 6, 2024

View reviewed changes

...n/scala/org/apache/pekko/persistence/testkit/internal/SnapshotStorageEmulatorExtension.scala Outdated Show resolved Hide resolved

pjfanning reviewed Oct 6, 2024

View reviewed changes

...stkit/src/test/scala/org/apache/pekko/persistence/testkit/scaladsl/RuntimeJournalsSpec.scala Show resolved Hide resolved

pjfanning reviewed Oct 6, 2024

View reviewed changes

...stkit/src/test/scala/org/apache/pekko/persistence/testkit/scaladsl/RuntimeJournalsSpec.scala Outdated Show resolved Hide resolved

pjfanning reviewed Oct 6, 2024

View reviewed changes

Address review comments

7b86e21

ptrdom requested a review from pjfanning October 7, 2024 16:54

Address review comments

d15e50b

ptrdom added 3 commits October 8, 2024 17:32

Add binary compat exclusions

c70c5cd

Fix test name

95c5996

Add Java API

4e02718

pjfanning reviewed Oct 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configuring persistence plugins at runtime for EventSourcedBehavior #1518

Configuring persistence plugins at runtime for EventSourcedBehavior #1518

ptrdom commented Oct 6, 2024

pjfanning Oct 6, 2024

ptrdom Oct 7, 2024

pjfanning Oct 7, 2024

ptrdom Oct 7, 2024

pjfanning Oct 7, 2024

pjfanning left a comment

pjfanning Oct 6, 2024

pjfanning Oct 6, 2024

ptrdom Oct 7, 2024

pjfanning Oct 7, 2024

ptrdom Oct 7, 2024

pjfanning Oct 7, 2024

ptrdom Oct 7, 2024

pjfanning Oct 7, 2024

pjfanning commented Oct 7, 2024

pjfanning Oct 8, 2024

	val pluginActorArgs: List[AnyRef] =
	try {
	Reflect.findConstructor(pluginClass, List(pluginConfig, configPath)) // will throw if not found
	List(pluginConfig, configPath)
	} catch {
	case NonFatal(_) =>
	try {
	Reflect.findConstructor(pluginClass, List(pluginConfig)) // will throw if not found
	List(pluginConfig)
	} catch {
	case NonFatal(_) => Nil
	} // otherwise use empty constructor
	}

		@@ -13,10 +13,12 @@

		package org.apache.pekko.persistence.typed.internal

		import com.typesafe.config.ConfigFactory

		@@ -107,14 +113,18 @@ private[pekko] final class BehaviorSetup[C, E, S](

		private var recoveryTimer: OptionVal[Cancellable] = OptionVal.None

		val recoveryEventTimeout: FiniteDuration = persistence

Configuring persistence plugins at runtime for EventSourcedBehavior #1518

Are you sure you want to change the base?

Configuring persistence plugins at runtime for EventSourcedBehavior #1518

Conversation

ptrdom commented Oct 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjfanning left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjfanning commented Oct 7, 2024

Choose a reason for hiding this comment