Skip to content
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

Akka.Cluster: Fix AK1004 warnings #7279

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

Aaronontheweb
Copy link
Member

@Aaronontheweb Aaronontheweb commented Jul 10, 2024

Changes

resolve a few warnings from https://getakka.net/articles/debugging/rules/AK1004.html inside Akka.Cluster

Checklist

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

@Aaronontheweb Aaronontheweb added akka-cluster rosyln-analyzer Issues that should be addressed via user-exposed Roslyn analyzers. labels Jul 10, 2024
Copy link
Member Author

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

Detailed my changes.

@@ -31,7 +31,7 @@ public interface IClusterMessage
/// <summary>
/// Cluster commands sent by the USER via <see cref="Cluster"/> extension.
/// </summary>
internal class ClusterUserAction
internal static class ClusterUserAction
Copy link
Member Author

Choose a reason for hiding this comment

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

This class is never instantiated, should be made static to resolve some compiler grumbling.

@@ -127,7 +120,7 @@ public Down(Address address)
/// <summary>
/// Command to join the cluster. Sent when a node wants to join another node (the receiver).
/// </summary>
internal sealed class InternalClusterAction
internal static class InternalClusterAction
Copy link
Member Author

Choose a reason for hiding this comment

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

This class is never instantiated, should be made static to resolve some compiler grumbling.

@@ -924,13 +917,13 @@ private void CreateChildren(Cluster cluster)
///
/// Actor used to power the guts of the Akka.Cluster membership and gossip protocols.
/// </summary>
internal class ClusterCoreDaemon : UntypedActor, IRequiresMessageQueue<IUnboundedMessageQueueSemantics>
internal sealed class ClusterCoreDaemon : UntypedActor, IRequiresMessageQueue<IUnboundedMessageQueueSemantics>, IWithTimers
Copy link
Member Author

Choose a reason for hiding this comment

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

Always try to seal classes that shouldn't be inherited. Also, added IWIthTimers support here as part of resolving AK1004.

{
private readonly Cluster _cluster;
/// <summary>
/// The current self-unique address.
/// </summary>
protected readonly UniqueAddress SelfUniqueAddress;
Copy link
Member Author

Choose a reason for hiding this comment

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

Resolved a compiler warning here (occurred after I sealed the class.)

@@ -1914,7 +1907,9 @@ public ReceiveGossipType ReceiveGossip(GossipEnvelope envelope)

if (comparison == VectorClock.Ordering.Concurrent)
{
_log.Debug(@"""Couldn't establish a causal relationship between ""remote"" gossip and ""local"" gossip - Remote[{0}] - Local[{1}] - merged them into [{2}]""",
_log.Debug("""
Copy link
Member Author

Choose a reason for hiding this comment

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

Use raw strings.

_cluster.Scheduler.ScheduleTellOnce(new TimeSpan(_cluster.Settings.GossipInterval.Ticks * 2 / 3), Self,
InternalClusterAction.GossipSpeedupTick.Instance, ActorRefs.NoSender);

Timers.StartSingleTimer("gossip-speedup-1", InternalClusterAction.GossipSpeedupTick.Instance, new TimeSpan(_cluster.Settings.GossipInterval.Ticks / 3));
Copy link
Member Author

Choose a reason for hiding this comment

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

Replace calls to scheduler with IWithTimers instead.

@@ -63,7 +63,7 @@ public static Props Props(Cluster getCluster)
/// <summary>
/// INTERNAL API
/// </summary>
internal class ClusterHeartbeatSender : ReceiveActor
internal class ClusterHeartbeatSender : ReceiveActor, IWithTimers
Copy link
Member Author

Choose a reason for hiding this comment

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

This class is inherited for testing purposes, no sealing. But did add IWithTimers.

Self,
new ExpectedFirstHeartbeat(to),
Self);
Timers.StartSingleTimer(to.Address, new ExpectedFirstHeartbeat(to), _cluster.Settings.HeartbeatExpectedResponseAfter);
Copy link
Member Author

Choose a reason for hiding this comment

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

Replace calls to scheduler with IWithTimers instead.

Copy link
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

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

LGTM

@Arkatufus Arkatufus enabled auto-merge (squash) July 10, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
akka-cluster rosyln-analyzer Issues that should be addressed via user-exposed Roslyn analyzers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants