-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add per-plugin recovery permitter actors #7448
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
// ----------------------------------------------------------------------- | ||
// <copyright file="MultipleRecoveryPermitterSpec.cs" company="Akka.NET Project"> | ||
// Copyright (C) 2009-2025 Lightbend Inc. <http://www.lightbend.com> | ||
// Copyright (C) 2013-2025 .NET Foundation <https://github.com/akkadotnet/akka.net> | ||
// </copyright> | ||
// ----------------------------------------------------------------------- | ||
|
||
using System.Threading.Tasks; | ||
using Akka.Actor; | ||
using Akka.Configuration; | ||
using FluentAssertions; | ||
using Xunit; | ||
|
||
namespace Akka.Persistence.Tests; | ||
|
||
public class MultipleRecoveryPermitterSpec : PersistenceSpec | ||
{ | ||
private readonly IActorRef _permitter1; | ||
private readonly IActorRef _permitter2; | ||
|
||
public MultipleRecoveryPermitterSpec() : base(ConfigurationFactory.ParseString($$""" | ||
akka.persistence | ||
{ | ||
# default global max recovery value | ||
max-concurrent-recoveries = 3 | ||
|
||
journal | ||
{ | ||
plugin = "akka.persistence.journal.inmem" | ||
inmem2 { | ||
# max recovery value override | ||
max-concurrent-recoveries = 20 | ||
class = "Akka.Persistence.Journal.MemoryJournal, Akka.Persistence" | ||
plugin-dispatcher = "akka.actor.default-dispatcher" | ||
} | ||
} | ||
|
||
# snapshot store plugin is NOT defined, things should still work | ||
snapshot-store.plugin = "akka.persistence.no-snapshot-store" | ||
snapshot-store.local.dir = "target/snapshots-"{{typeof(RecoveryPermitterSpec).FullName}}"} | ||
""")) | ||
{ | ||
var extension = Persistence.Instance.Apply(Sys); | ||
_permitter1 = extension.RecoveryPermitterFor(null); | ||
_permitter2 = extension.RecoveryPermitterFor("akka.persistence.journal.inmem2"); | ||
} | ||
|
||
[Fact(DisplayName = "Plugin max-concurrent-recoveries HOCON setting should override akka.persistence setting")] | ||
public async Task HoconOverrideTest() | ||
{ | ||
_permitter1.Tell(GetMaxPermits.Instance); | ||
await ExpectMsgAsync(3); | ||
|
||
_permitter2.Tell(GetMaxPermits.Instance); | ||
await ExpectMsgAsync(20); | ||
} | ||
|
||
[Fact(DisplayName = "Each plugin should have their own recovery permitter")] | ||
public void MultiRecoveryPermitterActorTest() | ||
{ | ||
_permitter1.Equals(_permitter2).Should().BeFalse(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -81,6 +81,7 @@ public abstract partial class Eventsourced : ActorBase, IPersistentIdentity, IPe | |
private readonly IStash _internalStash; | ||
private IActorRef _snapshotStore; | ||
private IActorRef _journal; | ||
private IActorRef _recoveryPermitter; | ||
private List<IPersistentEnvelope> _journalBatch = new(); | ||
private bool _isWriteInProgress; | ||
private long _sequenceNr; | ||
|
@@ -166,6 +167,8 @@ public IStash Stash | |
/// </summary> | ||
public IActorRef Journal => _journal ??= Extension.JournalFor(JournalPluginId); | ||
|
||
internal IActorRef RecoveryPermitter => _recoveryPermitter ??= Extension.RecoveryPermitterFor(JournalPluginId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lazy instantiation |
||
|
||
/// <summary> | ||
/// TBD | ||
/// </summary> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,6 @@ | |
using System; | ||
using System.Collections.Concurrent; | ||
using System.Linq; | ||
using System.Reflection; | ||
using System.Threading; | ||
using Akka.Actor; | ||
using Akka.Annotations; | ||
|
@@ -21,18 +20,21 @@ namespace Akka.Persistence | |
{ | ||
internal struct PluginHolder | ||
{ | ||
public PluginHolder(IActorRef @ref, EventAdapters adapters, Config config) | ||
public PluginHolder(IActorRef @ref, EventAdapters adapters, Config config, IActorRef recoveryPermitter) | ||
{ | ||
Ref = @ref; | ||
Adapters = adapters; | ||
Config = config; | ||
RecoveryPermitter = recoveryPermitter; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recovery permitter is also cached inside the |
||
} | ||
|
||
public IActorRef Ref { get; } | ||
|
||
public EventAdapters Adapters { get; } | ||
|
||
public Config Config { get; } | ||
|
||
public IActorRef RecoveryPermitter { get; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM |
||
} | ||
|
||
/// <summary> | ||
|
@@ -50,7 +52,6 @@ public class PersistenceExtension : IExtension | |
private readonly Lazy<string> _defaultJournalPluginId; | ||
private readonly Lazy<string> _defaultSnapshotPluginId; | ||
private readonly Lazy<IStashOverflowStrategy> _defaultInternalStashOverflowStrategy; | ||
private readonly Lazy<IActorRef> _recoveryPermitter; | ||
|
||
private readonly ConcurrentDictionary<string, Lazy<PluginHolder>> _pluginExtensionIds = new(); | ||
|
||
|
@@ -120,12 +121,6 @@ public PersistenceExtension(ExtendedActorSystem system) | |
_log.Info("Auto-starting snapshot store `{0}`", id); | ||
SnapshotStoreFor(id); | ||
}); | ||
|
||
_recoveryPermitter = new Lazy<IActorRef>(() => | ||
{ | ||
var maxPermits = _config.GetInt("max-concurrent-recoveries", 0); | ||
return _system.SystemActorOf(Akka.Persistence.RecoveryPermitter.Props(maxPermits), "recoveryPermitter"); | ||
}); | ||
} | ||
|
||
/// <summary> | ||
|
@@ -152,9 +147,10 @@ public string PersistenceId(IActorRef actor) | |
/// INTERNAL API: When starting many persistent actors at the same time the journal its data store is protected | ||
/// from being overloaded by limiting number of recoveries that can be in progress at the same time. | ||
/// </summary> | ||
internal IActorRef RecoveryPermitter() | ||
internal IActorRef RecoveryPermitterFor(string journalPluginId) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same design as |
||
{ | ||
return _recoveryPermitter.Value; | ||
var configPath = string.IsNullOrEmpty(journalPluginId) ? _defaultJournalPluginId.Value : journalPluginId; | ||
return PluginHolderFor(configPath, JournalFallbackConfigPath).RecoveryPermitter; | ||
} | ||
|
||
/// <summary> | ||
|
@@ -270,6 +266,17 @@ private PluginHolder PluginHolderFor(string configPath, string fallbackPath) | |
return pluginContainer.Value; | ||
} | ||
|
||
private static IActorRef CreateRecoveryPermitter(ExtendedActorSystem system, string configPath, Config pluginConfig) | ||
{ | ||
// backward compatibility | ||
// get the setting from the plugin path, if not found, default to the one defined in "akka.persistence" | ||
var maxPermits = pluginConfig.HasPath("max-concurrent-recoveries") | ||
? pluginConfig.GetInt("max-concurrent-recoveries") | ||
: system.Settings.Config.GetInt("akka.persistence.max-concurrent-recoveries"); | ||
|
||
return system.SystemActorOf(RecoveryPermitter.Props(maxPermits), $"recoveryPermitter-{configPath}"); | ||
} | ||
|
||
private static IActorRef CreatePlugin(ExtendedActorSystem system, string configPath, Config pluginConfig) | ||
{ | ||
var pluginActorName = configPath; | ||
|
@@ -303,8 +310,9 @@ private static PluginHolder NewPluginHolder(ExtendedActorSystem system, string c | |
var config = system.Settings.Config.GetConfig(configPath).WithFallback(system.Settings.Config.GetConfig(fallbackPath)); | ||
var plugin = CreatePlugin(system, configPath, config); | ||
var adapters = CreateAdapters(system, configPath); | ||
var recoveryPermitter = CreateRecoveryPermitter(system, configPath, config); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM |
||
|
||
return new PluginHolder(plugin, adapters, config); | ||
return new PluginHolder(plugin, adapters, config, recoveryPermitter); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The recovery permitter is being cached here instead of in the
Persistence
extension. This is lazily created when the plugin is initialized for the first time.