Should use `pekko` as HOCON config prefix rather than `akka` about pekko HOT 17 CLOSED

Regarding making pekko work with akka configs, another option (which I have to look into whether its possible) is to create a pekko-akka-config-compat module which can use typesafe config's substitution capabilities to just load pekko modules as akka ones.

I have a personal preference to abstract these things away into a separate module so the main codebase is clean.

from pekko.

1.1.x ?

So if you are referring to code that is dealing with translating from Akka to Pekko (i.e. rolling release upgrades) I do earnestly think this should only sit in 1.0.x branch, the main reason being is that once 1.0.x is released I envisage that our main branch will be the 1.1.x release branch and I believe there is merit in keeping that branch clean.

Furthermore, over time the 1.1.x branch will divert from mainstream Akka and having such functionality in the 1.1.x branch can give the misleading impression that you can do a roll over upgrade from Akka to Pekko 1.1.x which I don't think is desirable because we are then going to be shooting yourselves in the foot (both technically speaking in making sure that such upgrades still work and even legally).

In short, if we are only going to guarantee migrating from Akka to Pekko 1.0.x (which I think is sensible) then these compatibility features (i.e. handling any wire incompatibilities, custom code to translate from Akka HOCON to Pekko HOCON) should only sit in 1.0.x branch. This to me is a good compromise, people currently running Akka clusters can migrate to Pekko, i.e. Akka (current version) -> Akka 2.6 (if neccessary) -> Pekko 1.0.x -> change current Akka config/s to Pekko config -> upgrade to Pekko 1.1.x while at the same time we can keep our main branch free of any Akka compatibility code cruft.

The only exception to this is making the akka:// address configurable, I think there is general merit for this as a feature so I don't have a problem with that sitting in 1.1.x branch however only 1.0.x should have it as ["pekko", "akka"] for the default configuration where as 1.1.x should default to ["pekko"] (remember with HOCON you can always add more options to the list if you still want to use "akka"). To be clear, my though process in advocating for this is that aside from technical considerings, doing such a thing also sends a strong signal on what we are willing to support.

from pekko.

For 1.1.x, are we thinking to just remove the hook for config and defaulting back to the old behavior? That seems pretty dangerous because it might not be obvious to the user if the behavior changed after an upgrade. If we want to remove the hook, I'd suggest maybe deprecating it first, then in a future version have it throw an exception when you attempt to use it.

from pekko.

Right, I was thinking that too. Actually I think for most cases you could add pekko = ${akka} at the end of your config file. That wouldn't work for overrides using system properties but would probably get you most of the way there.

from pekko.

So after working on this PR #58 I don't think that the naive pekko = ${akka} will work. This is due to the fact that its typical in akka's typesafe to have values that are direct references to other typesafe keys, i.e.

akka.persistence.journal.inmem {
    # Class name of the plugin.
    class = "org.apache.pekko.persistence.journal.inmem.InmemJournal"
    # Dispatcher for the plugin actor.
    plugin-dispatcher = "akka.actor.default-dispatcher"

    # Turn this on to test serialization of the events
    test-serialization = off
}

and in this case akka.actor.default-dispatcher refers to an actual akka.actor.default-dispatcher setting, i.e.

    default-dispatcher {
      # Must be one of the following
      # Dispatcher, PinnedDispatcher, or a FQCN to a class inheriting
      # MessageDispatcherConfigurator with a public constructor with
      # both com.typesafe.config.Config parameter and
      # org.apache.pekko.dispatch.DispatcherPrerequisites parameters.
      # PinnedDispatcher must be used together with executor=thread-pool-executor.
      type = "Dispatcher"

      # Which kind of ExecutorService to use for this dispatcher
      # Valid options:
      #  - "default-executor" requires a "default-executor" section
      #  - "fork-join-executor" requires a "fork-join-executor" section
      #  - "thread-pool-executor" requires a "thread-pool-executor" section
      #  - "affinity-pool-executor" requires an "affinity-pool-executor" section
      #  - A FQCN of a class extending ExecutorServiceConfigurator
      executor = "default-executor"

      # This will be used if you have set "executor = "default-executor"".
      # If an ActorSystem is created with a given ExecutionContext, this
      # ExecutionContext will be used as the default executor for all
      # dispatchers in the ActorSystem configured with
      # executor = "default-executor". Note that "default-executor"
      # is the default value for executor, and therefore used if not
      # specified otherwise. If no ExecutionContext is given,
      # the executor configured in "fallback" will be used.
      default-executor {
        fallback = "fork-join-executor"
      }

      # This will be used if you have set "executor = "affinity-pool-executor""
      # Underlying thread pool implementation is org.apache.pekko.dispatch.affinity.AffinityPool.
      # This executor is classified as "ApiMayChange".
      affinity-pool-executor {
        # Min number of threads to cap factor-based parallelism number to
        parallelism-min = 4

        # The parallelism factor is used to determine thread pool size using the
        # following formula: ceil(available processors * factor). Resulting size
        # is then bounded by the parallelism-min and parallelism-max values.
        parallelism-factor = 0.8

        # Max number of threads to cap factor-based parallelism number to.
        parallelism-max = 64

        # Each worker in the pool uses a separate bounded MPSC queue. This value
        # indicates the upper bound of the queue. Whenever an attempt to enqueue
        # a task is made and the queue does not have capacity to accommodate
        # the task, the rejection handler created by the rejection handler specified
        # in "rejection-handler" is invoked.
        task-queue-size = 512

        # FQCN of the Rejection handler used in the pool.
        # Must have an empty public constructor and must
        # implement org.apache.pekko.actor.affinity.RejectionHandlerFactory.
        rejection-handler = "org.apache.pekko.dispatch.affinity.ThrowOnOverflowRejectionHandler"

        # Level of CPU time used, on a scale between 1 and 10, during backoff/idle.
        # The tradeoff is that to have low latency more CPU time must be used to be
        # able to react quickly on incoming messages or send as fast as possible after
        # backoff backpressure.
        # Level 1 strongly prefer low CPU consumption over low latency.
        # Level 10 strongly prefer low latency over low CPU consumption.
        idle-cpu-level = 5

        # FQCN of the org.apache.pekko.dispatch.affinity.QueueSelectorFactory.
        # The Class of the FQCN must have a public constructor with a
        # (com.typesafe.config.Config) parameter.
        # A QueueSelectorFactory create instances of org.apache.pekko.dispatch.affinity.QueueSelector,
        # that is responsible for determining which task queue a Runnable should be enqueued in.
        queue-selector = "org.apache.pekko.dispatch.affinity.FairDistributionHashCache"

        # When using the "org.apache.pekko.dispatch.affinity.FairDistributionHashCache" queue selector
        # internally the AffinityPool uses two methods to determine which task
        # queue to allocate a Runnable to:
        # - map based - maintains a round robin counter and a map of Runnable
        # hashcodes to queues that they have been associated with. This ensures
        # maximum fairness in terms of work distribution, meaning that each worker
        # will get approximately equal amount of mailboxes to execute. This is suitable
        # in cases where we have a small number of actors that will be scheduled on
        # the pool and we want to ensure the maximum possible utilization of the
        # available threads.
        # - hash based - the task - queue in which the runnable should go is determined
        # by using an uniformly distributed int to int hash function which uses the
        # hash code of the Runnable as an input. This is preferred in situations where we
        # have enough number of distinct actors to ensure statistically uniform
        # distribution of work across threads or we are ready to sacrifice the
        # former for the added benefit of avoiding map look-ups.
        fair-work-distribution {
          # The value serves as a threshold which determines the point at which the
          # pool switches from the first to the second work distribution schemes.
          # For example, if the value is set to 128, the pool can observe up to
          # 128 unique actors and schedule their mailboxes using the map based
          # approach. Once this number is reached the pool switches to hash based
          # task distribution mode. If the value is set to 0, the map based
          # work distribution approach is disabled and only the hash based is
          # used irrespective of the number of unique actors. Valid range is
          # 0 to 2048 (inclusive)
          threshold = 128
        }
      }
      // etc etc...

I think due to this plus other things (i.e. incompatibility in the wire protocol when changing from akka address to pekko) it makes more sense to just literally translate from akka to pekko and then only for the 1.0.x release branch of akka to write custom code that translates from akka to pekko. This allows current people who are running an Akka cluster to migrate to pekko and only having it in the 1.0.x branch means that we don't pollute the main codebase with these changes.

Thoughts?

from pekko.

I think due to this plus other things (i.e. incompatibility in the wire protocol when changing from akka address to pekko) it makes more sense to just literally translate from akka to pekko and then only for the 1.0.x release branch of akka to write custom code that translates from akka to pekko.

1.1.x ?

from pekko.

I think we should change all the config names to pekko. in v1.0.0. We can provide migration support with external tools, maybe Scalafix scripts.

from pekko.

I understand the reasoning of 1.1 already diverging.

I would leave some space for vanilla rename bugfix releases, before adding sparkles on top.

So don't put the config compat code in 1.0.1, but in e.g 1.0.10 or 1.0.100. That leaves 9 or 99 slots for fixes on the vanilla rename code and leaves the choice to pick up additional bugs due to compat code.

I for one rather hit the rename bugs than have those occluded by compat code.

I know this will be another one of those color and taste situations.

from pekko.

Thats perfectly reasonable for more, in fact I would say its preferable to have more iterative releases then doing it in one big bang.

@gmethvin Do you want to take this on?

from pekko.

Ideally we can do this without having to complicate the existing code much. If we can put the compat code in a separate module or make it customizable by the user, then I don't see any reason why we need to only have it in certain versions. We can just add a hook to customize the config loading and keep it there.

For example, we could define an interface PekkoConfigLoader.load() to replace our existing ConfigFactory.load():

trait PekkoConfigLoader {
  def load(): Config
}
object PekkoConfigLoader {
  def load(): Config = sys.props.get("pekko.config-loader") match {
    case Some(className) =>
      Class.forName(className).getConstructor().newInstance().asInstanceOf[PekkoConfigLoader].load()
    case None =>
      ConfigFactory.load()
  }
}

Then anyone can define their own class that extends this trait and set a system property pekko.config-loader=com.example.MyConfigLoaderImpl. That way we could include this logic in a separate module and have the user configure it themselves.

I'm happy to take this on if we can agree on the right approach here.

from pekko.

If it requires a separate module to activate, that's probably sufficient shielding. I agree this proposal is simple enough to state that without the module it equates to the original.

from pekko.

Just to expand on what I was suggesting above: it should be possible to generate an equivalent Pekko config from an existing Akka config using a custom config loader. We can handle the dispatcher references by also cloning the generated pekko config to the akka config prefix. In other words we'd make akka and pekko act like aliases of each other.

We really should do this at each level of config loading: reference.conf, application.conf, and system/env overrides. For example, we want akka settings in application.conf to override pekko settings in reference.conf. Within the same config file, it probably makes sense to just prefer pekko over akka and assume users will migrate everything in the file all at once, though it gets more complicated when you consider includes.

The loading logic might look something like this:

def alias(config: Config): Config = {
  val pekkoCloned =
    if (config.hasPath("pekko"))
      config.getConfig("pekko").atPath("pekko")
        .withFallback(config.getConfig("pekko").atPath("akka"))
    else ConfigFactory.empty
  val akkaCloned =
    if (config.hasPath("akka"))
      config.getConfig("akka").atPath("pekko")
        .withFallback(config.getConfig("akka").atPath("akka"))
    else ConfigFactory.empty
  config.withoutPath("pekko").withoutPath("akka").withFallback(pekkoCloned).withFallback(akkaCloned)
}

alias(ConfigFactory.defaultOverrides())
  .withFallback(alias(ConfigFactory.defaultApplication()))
  .withFallback(alias(ConfigFactory.defaultReference()))
  .resolve()

I think this is probably good enough for most purposes, assuming it works as intended.

from pekko.

I still have a preference for only having this in the 1.0.x branch, I think there is a lot of merit gained in having the main codebase as simple as possible (plus the other concerns I stated earlier). However this decision is something that will likely need to be voted on and it shouldn't stop prototyping to figure out the best path forward.

The loading logic might look something like this:

Is this going to work with the problem I described at #54 (comment) ?

from pekko.

I still have a preference for only having this in the 1.0.x branch, I think there is a lot of merit gained in having the main codebase as simple as possible (plus the other concerns I stated earlier).

Sure, that's why I'd want to make sure we keep the changes in Pekko simple (e.g. adding a config loader hook) and put the complex logic somewhere else. Still maybe we only want to keep this hook for compatibility in 1.0.x, but I don't really have a sense of how it would be used in the real world, so it's hard to tell if that's a good decision.

Is this going to work with the problem I described at #54 (comment) ?

I think it should handle that case, yes, because it clones the final merged Pekko config to the akka prefix as well. So in the example you gave, Pekko would read the value for pekko.persistence.journal.inmem.plugin-dispatcher as akka.actor-default-dispatcher, since that's just a string in the config. It would then look up the config at akka.actor.default-dispatcher which would have the same value as pekko.actor.default-dispatcher.

from pekko.

If the hook is really simple and defaults to the classic behavior I think it's reasonable to have in 1.0.x releases.
With the compat code split out to a completely separate codebase, so it's opt-in. It also means the compat code ca have a separate release cycle.

I'd argue you can use .conf files, or Java SPI so that the act of including the compat module on the classpath activates it (be it that you can also explicitly deactivate it).

from pekko.

Also I noticed Typesafe Config already has a mechanism to customize the config loading called ConfigLoadingStrategy, but unfortunately that only works for the application config, not system properties or reference config, so it's not quite what we want. We might be able to hack it to do what we want though...

from pekko.

I'm working on the akka to pekko config change in #63.

I noticed also in some of the serialization configs there are values that contain akka that need to be changed, for example: https://github.com/apache/incubator-pekko/blob/9be4cad14fdab9702a8db1c657a3fcac135bba48/akka-cluster/src/main/resources/reference.conf#L372

That would require some more specific logic to support in the akka compatibility layer.

from pekko.