Skip to content

Add per-plugin recovery permitter actors - #7448

Merged
Aaronontheweb merged 4 commits into
akkadotnet:devfrom
Arkatufus:Add-per-plugin-recovery-permitter
Jan 8, 2025
Merged

Add per-plugin recovery permitter actors#7448
Aaronontheweb merged 4 commits into
akkadotnet:devfrom
Arkatufus:Add-per-plugin-recovery-permitter

Conversation

@Arkatufus

@Arkatufus Arkatufus commented Jan 8, 2025

Copy link
Copy Markdown
Contributor

Fixes #7447

Changes

All changes are applied to internal APIs.

  • Move recovery permitter cache from global Persistence extension to each EventSourced actors.
  • RecoveryPermitter actor is now lazily created and stored inside persistence PluginHolder
  • Add option to declare "max-concurrent-recoveries" HOCON setting in each persistence plugin that, if declared, will override the global "akka.persistence.max-concurrent-recoveries"

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design looks good - but we should add some specs:

  1. Validate that we look for a plugin-specific max-concurrent-recoveries and that that can override the global default
  2. Validate, with a quick test against the Persistence extension itself, that multiple recovery permitter actors are created if multiple journals are defined.

No need for a big End2End integration spec, I think that should be fine.

/// </summary>
public IActorRef Journal => _journal ??= Extension.JournalFor(JournalPluginId);

internal IActorRef RecoveryPermitter => _recoveryPermitter ??= Extension.RecoveryPermitterFor(JournalPluginId);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


public Config Config { get; }

public IActorRef RecoveryPermitter { get; }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Arkatufus Arkatufus left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review

private readonly IStash _internalStash;
private IActorRef _snapshotStore;
private IActorRef _journal;
private IActorRef _recoveryPermitter;

Copy link
Copy Markdown
Contributor Author

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.

/// </summary>
public IActorRef Journal => _journal ??= Extension.JournalFor(JournalPluginId);

internal IActorRef RecoveryPermitter => _recoveryPermitter ??= Extension.RecoveryPermitterFor(JournalPluginId);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lazy instantiation

Ref = @ref;
Adapters = adapters;
Config = config;
RecoveryPermitter = recoveryPermitter;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recovery permitter is also cached inside the PluginHolder so that it can be looked up.

/// 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)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same design as AdaptersFor() to look up recovery permitter actor per plugin basis

@Arkatufus

Copy link
Copy Markdown
Contributor Author

Done adding spec

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[PERF] Akka.Cluster.Sharding remember-entity recovery timeouts for large entity counts

2 participants