From f7f118eba17270c55e2a8c177a01cda545514cda Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 16 Apr 2019 17:16:27 -0500 Subject: [PATCH 01/36] cleaning up comments and code --- src/core/Akka.Remote/EndpointManager.cs | 2 +- src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs | 2 +- src/core/Akka/Actor/ActorPath.cs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index acd8e240866..a63a41e1939 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -671,7 +671,7 @@ protected override void PostStop() { // Remaining running endpoints are children, so they will clean up themselves. // We still need to clean up any remaining transports because handles might be in mailboxes, and for example - // Netty is not part of the actor hierarchy, so its handles will not be cleaned up if no actor is taking + // DotNetty is not part of the actor hierarchy, so its handles will not be cleaned up if no actor is taking // responsibility of them (because they are sitting in a mailbox). _log.Error("Remoting system has been terminated abruptly. Attempting to shut down transports"); foreach (var t in _transportMapping.Values) diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index 78bff13ae25..aa9d49aaaf2 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -718,7 +718,7 @@ public override string ToString() internal class ForbiddenUidReason { } /// - /// TBD + /// INTERNAL API. /// internal class ProtocolStateActor : FSM { diff --git a/src/core/Akka/Actor/ActorPath.cs b/src/core/Akka/Actor/ActorPath.cs index c74d8be7b50..5a5322d9db1 100644 --- a/src/core/Akka/Actor/ActorPath.cs +++ b/src/core/Akka/Actor/ActorPath.cs @@ -635,9 +635,9 @@ public override ActorPath Root get { var current = _parent; - while (current is ChildActorPath) + while (current is ChildActorPath child) { - current = ((ChildActorPath)current)._parent; + current = child._parent; } return current.Root; } From 66efd08aab924e7eb07d32fa250971d911ddc23f Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 24 Apr 2019 17:53:38 -0500 Subject: [PATCH 02/36] cleaned up some EndpointManager code --- src/core/Akka.Remote/EndpointManager.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index a63a41e1939..ca56f74f7a9 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -773,17 +773,16 @@ protected void Accepting() //Stop writers var policy = Tuple.Create(_endpoints.WritableEndpointWithPolicyFor(quarantine.RemoteAddress), quarantine.Uid); - if (policy.Item1 is Pass && policy.Item2 == null) + if (policy.Item1 is Pass pass && policy.Item2 == null) { - var endpoint = policy.Item1.AsInstanceOf().Endpoint; + var endpoint = pass.Endpoint; Context.Stop(endpoint); _log.Warning("Association to [{0}] with unknown UID is reported as quarantined, but " + "address cannot be quarantined without knowing the UID, gating instead for {1} ms.", quarantine.RemoteAddress, _settings.RetryGateClosedFor.TotalMilliseconds); _endpoints.MarkAsFailed(endpoint, Deadline.Now + _settings.RetryGateClosedFor); } - else if (policy.Item1 is Pass && policy.Item2 != null) + else if (policy.Item1 is Pass pass && policy.Item2 != null) { - var pass = (Pass)policy.Item1; var uidOption = pass.Uid; var quarantineUid = policy.Item2; if (uidOption == quarantineUid) @@ -803,9 +802,8 @@ protected void Accepting() //the quarantine uid has lost the race with some failure, do nothing } } - else if (policy.Item1 is WasGated && policy.Item2 != null) + else if (policy.Item1 is WasGated wg && policy.Item2 != null) { - var wg = (WasGated)policy.Item1; if (wg.RefuseUid == policy.Item2) _endpoints.RegisterWritableEndpointRefuseUid(quarantine.RemoteAddress, policy.Item2.Value); } From 1458101c9747dd9e489b539a69c6a2642039d63d Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 24 Apr 2019 18:11:49 -0500 Subject: [PATCH 03/36] working on porting relevant spec --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index 66cc547c67f..d7cb8787ee4 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -457,6 +457,13 @@ public async Task Bug_884_Remoting_must_support_reply_to_child_of_Routee() Assert.Equal(reporter, msg.Item2); } + [Fact] + public void Should_properly_quarantine_stashed_inbound_connections() + { + var localAddress = new Address("akka.test", "system1", "localhost", 1); + var rawLocalAddress = new Address("test", "system1", "localhost", 1); + } + [Fact] public void Drop_sent_messages_over_payload_size() { From 58f025260c18351a1e343b50778f8a410e203a75 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 24 Apr 2019 19:14:56 -0500 Subject: [PATCH 04/36] adding some missing test methods to RemotingSpec --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 32 ++++++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index d7cb8787ee4..0cc6f59b204 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -14,6 +14,7 @@ using Akka.Remote.Transport; using Akka.Routing; using Akka.TestKit; +using Akka.TestKit.TestEvent; using Akka.Util; using Akka.Util.Internal; using Google.Protobuf; @@ -40,6 +41,8 @@ public RemotingSpec(ITestOutputHelper helper) : base(GetConfig(), helper) _remote = _remoteSystem.ActorOf(Props.Create(), "echo"); _here = Sys.ActorSelection("akka.test://remote-sys@localhost:12346/user/echo"); + + AtStartup(); } private static string GetConfig() @@ -458,10 +461,26 @@ public async Task Bug_884_Remoting_must_support_reply_to_child_of_Routee() } [Fact] - public void Should_properly_quarantine_stashed_inbound_connections() + public void Properly_quarantine_stashed_inbound_connections() { var localAddress = new Address("akka.test", "system1", "localhost", 1); var rawLocalAddress = new Address("test", "system1", "localhost", 1); + var remoteAddress = new Address("akka.test", "system2", "localhost", 2); + var rawRemoteAddress = new Address("test", "system2", "localhost", 2); + var remoteUID = 16; + + var config = ConfigurationFactory.ParseString(@" + akka.remote.enabled-transports = [""akka.remote.test""] + akka.remote.retry-gate-closed-for = 5s + akka.remote.log-remote-lifecycle-events = on + + akka.remote.test { + registry-key = JMeMndLLsw + local-address = """ + $"test://{localAddress.System}@{localAddress.Host}:{localAddress.Port}" + @""" + }").WithFallback(_remoteSystem.Settings.Config); + + var thisSystem = ActorSystem.Create("this-system", config); + } [Fact] @@ -569,10 +588,17 @@ private void VerifySend(object msg, Action afterSend) private void AtStartup() { - //TODO need to implement test filters first + MuteSystem(Sys); + _remoteSystem.EventStream.Publish(EventFilter.Error(start: "AssociationError").Mute()); + _remoteSystem.EventStream.Publish(EventFilter.Exception().Mute()); } - + private void MuteSystem(ActorSystem system) + { + system.EventStream.Publish(EventFilter.Error(start: "AssociationError").Mute()); + system.EventStream.Publish(EventFilter.Warning(start: "AssociationError").Mute()); + system.EventStream.Publish(EventFilter.Warning(contains: "dead letter").Mute()); + } private Address Addr(ActorSystem system, string proto) { From 31cf1475d9c3033d1999766361d21d4d94ce657a Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 24 Apr 2019 19:40:47 -0500 Subject: [PATCH 05/36] completed port of Properly_quarantine_stashed_inbound_connections --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 79 +++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index 0cc6f59b204..d53f31cef73 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -480,7 +480,67 @@ public void Properly_quarantine_stashed_inbound_connections() }").WithFallback(_remoteSystem.Settings.Config); var thisSystem = ActorSystem.Create("this-system", config); - + MuteSystem(thisSystem); + + try + { + // Set up a mock remote system using the test transport + var registry = AssociationRegistry.Get("JMeMndLLsw"); + var remoteTransport = new TestTransport(rawRemoteAddress, registry); + var remoteTransportProbe = CreateTestProbe(); + + registry.RegisterTransport(remoteTransport, Task.FromResult + (new ActorAssociationEventListener(remoteTransportProbe))); + + // Hijack associations through the test transport + AwaitCondition(() => registry.TransportsReady(rawLocalAddress, rawRemoteAddress)); + var testTransport = registry.TransportFor(rawLocalAddress).Item1; + testTransport.WriteBehavior.PushConstant(true); + + // Force an outbound associate on the real system (which we will hijack) + // we send no handshake packet, so this remains a pending connection + var dummySelection = thisSystem.ActorSelection(ActorPath.Parse(remoteAddress + "/user/noonethere")); + dummySelection.Tell("ping", Sys.DeadLetters); + + var remoteHandle = remoteTransportProbe.ExpectMsg(); + remoteHandle.Association.ReadHandlerSource.TrySetResult((IHandleEventListener)(new ActionHandleEventListener(ev => {}))); + + // Now we initiate an emulated inbound connection to the real system + var inboundHandleProbe = CreateTestProbe(); + var inboundHandleTask = remoteTransport.Associate(rawLocalAddress); + inboundHandleTask.Wait(TimeSpan.FromSeconds(3)); + var inboundHandle = inboundHandleTask.Result; + inboundHandle.ReadHandlerSource.TrySetResult(new ActorHandleEventListener(inboundHandleProbe)); + + AwaitAssert(() => + { + registry.GetRemoteReadHandlerFor(inboundHandle.AsInstanceOf()); + }); + + var pduCodec = new AkkaPduProtobuffCodec(thisSystem); + + var handshakePacket = pduCodec.ConstructAssociate(new HandshakeInfo(remoteAddress, remoteUID)); + + // Finish the inbound handshake so now it is handed up to Remoting + inboundHandle.Write(handshakePacket); + + // No disassociation now, the connection is still stashed + inboundHandleProbe.ExpectNoMsg(1000); + + // Quarantine unrelated connection + RARP.For(thisSystem).Provider.Quarantine(remoteAddress, -1); + inboundHandleProbe.ExpectNoMsg(1000); + + // Quarantine the connection + RARP.For(thisSystem).Provider.Quarantine(remoteAddress, remoteUID); + + // Even though the connection is stashed it will be disassociated + inboundHandleProbe.ExpectMsg(); + } + finally + { + Shutdown(thisSystem); + } } [Fact] @@ -755,6 +815,23 @@ public T Resolve(object[] args) } } + class ActionHandleEventListener : IAssociationEventListener + { + private readonly Action _handler; + + public ActionHandleEventListener() : this(ev => { }) { } + + public ActionHandleEventListener(Action handler) + { + _handler = handler; + } + + public void Notify(IAssociationEvent ev) + { + _handler(ev); + } + } + #endregion } From eb05ef3434efb35d9f8edf3e07e7dedc91f54c46 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 24 Apr 2019 19:43:45 -0500 Subject: [PATCH 06/36] fixed endpoint manager issue --- src/core/Akka.Remote/EndpointManager.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index ca56f74f7a9..b082549cdaa 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -781,18 +781,18 @@ protected void Accepting() "address cannot be quarantined without knowing the UID, gating instead for {1} ms.", quarantine.RemoteAddress, _settings.RetryGateClosedFor.TotalMilliseconds); _endpoints.MarkAsFailed(endpoint, Deadline.Now + _settings.RetryGateClosedFor); } - else if (policy.Item1 is Pass pass && policy.Item2 != null) + else if (policy.Item1 is Pass p && policy.Item2 != null) { - var uidOption = pass.Uid; + var uidOption = p.Uid; var quarantineUid = policy.Item2; if (uidOption == quarantineUid) { _endpoints.MarkAsQuarantined(quarantine.RemoteAddress, quarantineUid.Value, Deadline.Now + _settings.QuarantineDuration); _eventPublisher.NotifyListeners(new QuarantinedEvent(quarantine.RemoteAddress, quarantineUid.Value)); - Context.Stop(pass.Endpoint); + Context.Stop(p.Endpoint); } // or it does not match with the UID to be quarantined - else if (!uidOption.HasValue && pass.RefuseUid != quarantineUid) + else if (!uidOption.HasValue && p.RefuseUid != quarantineUid) { // the quarantine uid may be got fresh by cluster gossip, so update refuseUid for late handle when the writer got uid _endpoints.RegisterWritableEndpointRefuseUid(quarantine.RemoteAddress, quarantineUid.Value); From 43d15aa7accf174d7678e262ec158400ea31510a Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 24 Apr 2019 20:10:15 -0500 Subject: [PATCH 07/36] fixed issues with ActionHandleEventListener --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index d53f31cef73..8c63db92288 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -41,8 +41,6 @@ public RemotingSpec(ITestOutputHelper helper) : base(GetConfig(), helper) _remote = _remoteSystem.ActorOf(Props.Create(), "echo"); _here = Sys.ActorSelection("akka.test://remote-sys@localhost:12346/user/echo"); - - AtStartup(); } private static string GetConfig() @@ -646,7 +644,7 @@ private void VerifySend(object msg, Action afterSend) } } - private void AtStartup() + protected override void AtStartup() { MuteSystem(Sys); _remoteSystem.EventStream.Publish(EventFilter.Error(start: "AssociationError").Mute()); @@ -815,18 +813,18 @@ public T Resolve(object[] args) } } - class ActionHandleEventListener : IAssociationEventListener + class ActionHandleEventListener : IHandleEventListener { - private readonly Action _handler; + private readonly Action _handler; public ActionHandleEventListener() : this(ev => { }) { } - public ActionHandleEventListener(Action handler) + public ActionHandleEventListener(Action handler) { _handler = handler; } - public void Notify(IAssociationEvent ev) + public void Notify(IHandleEvent ev) { _handler(ev); } From b3a36e18ee559b9db87c9266d0cfe8938176cc4a Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 09:55:56 -0500 Subject: [PATCH 08/36] fixed NRE --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index 8c63db92288..7be66ec30ee 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -41,6 +41,8 @@ public RemotingSpec(ITestOutputHelper helper) : base(GetConfig(), helper) _remote = _remoteSystem.ActorOf(Props.Create(), "echo"); _here = Sys.ActorSelection("akka.test://remote-sys@localhost:12346/user/echo"); + + AtStartup(); } private static string GetConfig() @@ -643,8 +645,12 @@ private void VerifySend(object msg, Action afterSend) bigBounceOther.Tell(PoisonPill.Instance); } } - - protected override void AtStartup() + + /// + /// Have to hide other method otherwise we get an NRE due to base class + /// constructor being called first. + /// + protected new void AtStartup() { MuteSystem(Sys); _remoteSystem.EventStream.Publish(EventFilter.Error(start: "AssociationError").Mute()); From b9ef45c4c700ddbfc33f08e2210ab3c53bc924af Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 11:39:05 -0500 Subject: [PATCH 09/36] porting https://github.com/akka/akka/pull/23617 --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 4 +- src/core/Akka.Remote/EndpointManager.cs | 54 +++----------- src/core/Akka.Remote/EndpointRegistry.cs | 86 ++++++++++------------ 3 files changed, 54 insertions(+), 90 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index 7be66ec30ee..c28af6a878d 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -17,10 +17,12 @@ using Akka.TestKit.TestEvent; using Akka.Util; using Akka.Util.Internal; +using FluentAssertions; using Google.Protobuf; using Xunit; using Xunit.Abstractions; using Nito.AsyncEx; +using ThreadLocalRandom = Akka.Util.ThreadLocalRandom; namespace Akka.Remote.Tests { @@ -514,7 +516,7 @@ public void Properly_quarantine_stashed_inbound_connections() AwaitAssert(() => { - registry.GetRemoteReadHandlerFor(inboundHandle.AsInstanceOf()); + registry.GetRemoteReadHandlerFor(inboundHandle.AsInstanceOf()).Should().NotBeNull(); }); var pduCodec = new AkkaPduProtobuffCodec(thisSystem); diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index b082549cdaa..33f4917a90f 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -30,7 +30,7 @@ internal class EndpointManager : ReceiveActor, IRequiresMessageQueue - /// TBD + /// Defines how we're going to treat incoming and outgoing connections for specific endpoints /// public abstract class EndpointPolicy { @@ -39,10 +39,6 @@ public abstract class EndpointPolicy /// public readonly bool IsTombstone; - /// - /// TBD - /// - /// TBD protected EndpointPolicy(bool isTombstone) { IsTombstone = isTombstone; @@ -59,29 +55,22 @@ public sealed class Pass : EndpointPolicy /// /// TBD /// TBD - /// TBD - public Pass(IActorRef endpoint, int? uid, int? refuseUid) + public Pass(IActorRef endpoint, int? uid) : base(false) { Uid = uid; Endpoint = endpoint; - RefuseUid = refuseUid; } /// - /// TBD + /// The actor who owns the current endpoint /// public IActorRef Endpoint { get; private set; } /// - /// TBD + /// The endpoint UID, if it's currently known /// public int? Uid { get; private set; } - - /// - /// TBD - /// - public int? RefuseUid { get; private set; } } /// @@ -95,42 +84,16 @@ public sealed class Gated : EndpointPolicy /// /// TBD /// TBD - public Gated(Deadline deadline, int? refuseUid) + public Gated(Deadline deadline) : base(true) { TimeOfRelease = deadline; - RefuseUid = refuseUid; } /// /// TBD /// public Deadline TimeOfRelease { get; private set; } - - /// - /// TBD - /// - public int? RefuseUid { get; private set; } - } - - /// - /// Used to indicated that a node was previously. - /// - public sealed class WasGated : EndpointPolicy - { - /// - /// TBD - /// - /// TBD - public WasGated(int? refuseUid) : base(false) - { - RefuseUid = refuseUid; - } - - /// - /// TBD - /// - public int? RefuseUid { get; private set; } } /// @@ -829,7 +792,12 @@ protected void Accepting() else if (readPolicy.Item1?.Item1 != null && quarantine.Uid != null && readPolicy.Item1?.Item2 == quarantine.Uid) { Context.Stop(readPolicy.Item1.Item1); } else { } // nothing to stop - bool MatchesQuarantine(AkkaProtocolHandle handle) => handle.RemoteAddress.Equals(quarantine.RemoteAddress) && quarantine.Uid == handle.HandshakeInfo.Uid; + bool MatchesQuarantine(AkkaProtocolHandle handle) + { + + return handle.RemoteAddress.Equals(quarantine.RemoteAddress) && + quarantine.Uid == handle.HandshakeInfo.Uid; + } // Stop all matching pending read handoffs _pendingReadHandoffs = _pendingReadHandoffs.Where(x => diff --git a/src/core/Akka.Remote/EndpointRegistry.cs b/src/core/Akka.Remote/EndpointRegistry.cs index 4155b9323c1..34b8edc55a0 100644 --- a/src/core/Akka.Remote/EndpointRegistry.cs +++ b/src/core/Akka.Remote/EndpointRegistry.cs @@ -18,6 +18,7 @@ namespace Akka.Remote /// internal class EndpointRegistry { + private readonly Dictionary> _addressToRefuseUid = new Dictionary>(); private readonly Dictionary> _addressToReadonly = new Dictionary>(); private Dictionary _addressToWritable = @@ -27,75 +28,67 @@ internal class EndpointRegistry private readonly Dictionary _writableToAddress = new Dictionary(); /// - /// TBD + /// Registers a new writable endpoint with the system. /// - /// TBD - /// TBD - /// TBD - /// TBD + /// The remote address.> + /// The local endpoint actor who owns this connection. + /// The UID of the remote actor system. Can be null. /// /// This exception is thrown when the specified does not have /// an of /// in the registry. /// - /// TBD - public IActorRef RegisterWritableEndpoint(Address address, IActorRef endpoint, int? uid, int? refuseUid) + /// The actor reference. + public IActorRef RegisterWritableEndpoint(Address address, IActorRef endpoint, int? uid) { - _addressToWritable.TryGetValue(address, out var existing); - - var pass = existing as EndpointManager.Pass; - if (pass != null) // if we already have a writable endpoint.... + if (_addressToWritable.TryGetValue(address, out var existing)) { - throw new ArgumentException($"Attempting to overwrite existing endpoint {pass.Endpoint} with {endpoint}"); + if (existing is EndpointManager.Pass pass) // if we already have a writable endpoint.... + { + throw new ArgumentException($"Attempting to overwrite existing endpoint {pass.Endpoint} with {endpoint}"); + } } - _addressToWritable[address] = new EndpointManager.Pass(endpoint, uid, refuseUid); + // note that this overwrites Quarantine marker, + // but that is ok since we keep the quarantined uid in addressToRefuseUid + _addressToWritable[address] = new EndpointManager.Pass(endpoint, uid); _writableToAddress[endpoint] = address; return endpoint; } /// - /// TBD + /// Sets the UID for an existing policy. /// - /// TBD - /// TBD + /// The address of the remote system. + /// The UID of the remote system. public void RegisterWritableEndpointUid(Address remoteAddress, int uid) { if (_addressToWritable.TryGetValue(remoteAddress, out var existing)) { - var pass = existing as EndpointManager.Pass; - if (pass != null) - _addressToWritable[remoteAddress] = new EndpointManager.Pass(pass.Endpoint, uid, pass.RefuseUid); + if (existing is EndpointManager.Pass pass) + _addressToWritable[remoteAddress] = new EndpointManager.Pass(pass.Endpoint, uid); // if the policy is not Pass, then the GotUid might have lost the race with some failure } } /// - /// TBD + /// Record a "refused" UID for a remote system that has been quarantined. /// - /// TBD - /// TBD - public void RegisterWritableEndpointRefuseUid(Address remoteAddress, int refuseUid) + /// The remote address of the quarantined system. + /// The refused UID of the remote system. + /// The timeframe for releasing quarantine. + public void RegisterWritableEndpointRefuseUid(Address remoteAddress, int refuseUid, Deadline timeOfRelease) { - if (_addressToWritable.TryGetValue(remoteAddress, out var existing)) - { - var pass = existing as EndpointManager.Pass; - if (pass != null) - _addressToWritable[remoteAddress] = new EndpointManager.Pass(pass.Endpoint, pass.Uid, refuseUid); - else if (existing is EndpointManager.Gated) - _addressToWritable[remoteAddress] = new EndpointManager.Gated(((EndpointManager.Gated)existing).TimeOfRelease, refuseUid); - else if (existing is EndpointManager.WasGated) - _addressToWritable[remoteAddress] = new EndpointManager.WasGated(refuseUid); - } + _addressToRefuseUid[remoteAddress] = Tuple.Create(refuseUid, timeOfRelease); } /// - /// TBD + /// Registers a read-only endpoint. /// - /// TBD - /// TBD - /// TBD - /// TBD + /// The remote address.> + /// The local endpoint actor who owns this connection. + /// The UID of the remote actor system. Can be null. + /// The actor reference. public IActorRef RegisterReadOnlyEndpoint(Address address, IActorRef endpoint, int uid) { _addressToReadonly[address] = Tuple.Create(endpoint, uid); @@ -104,9 +97,9 @@ public IActorRef RegisterReadOnlyEndpoint(Address address, IActorRef endpoint, i } /// - /// TBD + /// Unregisters an endpoint from the registry. /// - /// TBD + /// The actor who owns the endpoint. public void UnregisterEndpoint(IActorRef endpoint) { if (IsWritable(endpoint)) @@ -120,6 +113,7 @@ public void UnregisterEndpoint(IActorRef endpoint) else _addressToWritable.Remove(address); _writableToAddress.Remove(endpoint); + // leave the refuseUid } else if (IsReadOnly(endpoint)) { @@ -129,10 +123,10 @@ public void UnregisterEndpoint(IActorRef endpoint) } /// - /// TBD + /// Get the endpoint address for the selected endpoint writer actor. /// - /// TBD - /// TBD + /// The endpoint writer actor reference. + /// The remote system address owned by . public Address AddressForWriter(IActorRef writer) { // Needs to return null if the key is not in the dictionary, instead of throwing. @@ -141,10 +135,10 @@ public Address AddressForWriter(IActorRef writer) } /// - /// TBD + /// Gets a read-only endpoint for the given address, if it exists. /// - /// TBD - /// TBD + /// The remote address to check. + /// A tuple containing the actor reference and the remote system UID, if they exist. Otherwise null. public Tuple ReadOnlyEndpointFor(Address address) { _addressToReadonly.TryGetValue(address, out var tmp); From f398de50f608c73f618f3e1b14cbfed7b3336529 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 11:56:49 -0500 Subject: [PATCH 10/36] still working on updating EndpointRegistry --- .../Akka.Remote.Tests/EndpointRegistrySpec.cs | 2 +- src/core/Akka.Remote/EndpointManager.cs | 4 +- src/core/Akka.Remote/EndpointRegistry.cs | 104 +++++++++--------- 3 files changed, 56 insertions(+), 54 deletions(-) diff --git a/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs b/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs index 2a14e3a5ae3..ec205d5ed10 100644 --- a/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs +++ b/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs @@ -213,7 +213,7 @@ public void EndpointRegister_should_not_report_endpoint_as_writeable_if_no_Pass_ reg.RegisterWritableEndpoint(address1, TestActor, 43, 42); // restarted Assert.True(reg.IsWritable(TestActor)); // pass reg.MarkAsQuarantined(address1, 43, deadline); - Assert.False(reg.HasWriteableEndpointFor(address1)); // Quarantined + Assert.False(reg.HasWritableEndpointFor(address1)); // Quarantined } } } diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index 33f4917a90f..f8e9915cb7a 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -1134,7 +1134,7 @@ private void RemovePendingReader(IActorRef takingOverFrom, AkkaProtocolHandle wi private void CreateAndRegisterEndpoint(AkkaProtocolHandle handle, int? refuseUid) { - var writing = _settings.UsePassiveConnections && !_endpoints.HasWriteableEndpointFor(handle.RemoteAddress); + var writing = _settings.UsePassiveConnections && !_endpoints.HasWritableEndpointFor(handle.RemoteAddress); _eventPublisher.NotifyListeners(new AssociatedEvent(handle.LocalAddress, handle.RemoteAddress, true)); var endpoint = CreateEndpoint( handle.RemoteAddress, @@ -1152,7 +1152,7 @@ private void CreateAndRegisterEndpoint(AkkaProtocolHandle handle, int? refuseUid else { _endpoints.RegisterReadOnlyEndpoint(handle.RemoteAddress, endpoint, handle.HandshakeInfo.Uid); - if (!_endpoints.HasWriteableEndpointFor(handle.RemoteAddress)) + if (!_endpoints.HasWritableEndpointFor(handle.RemoteAddress)) _endpoints.RemovePolicy(handle.RemoteAddress); } } diff --git a/src/core/Akka.Remote/EndpointRegistry.cs b/src/core/Akka.Remote/EndpointRegistry.cs index 34b8edc55a0..ad29e4273df 100644 --- a/src/core/Akka.Remote/EndpointRegistry.cs +++ b/src/core/Akka.Remote/EndpointRegistry.cs @@ -18,7 +18,7 @@ namespace Akka.Remote /// internal class EndpointRegistry { - private readonly Dictionary> _addressToRefuseUid = new Dictionary>(); + private Dictionary> _addressToRefuseUid = new Dictionary>(); private readonly Dictionary> _addressToReadonly = new Dictionary>(); private Dictionary _addressToWritable = @@ -146,31 +146,31 @@ public Tuple ReadOnlyEndpointFor(Address address) } /// - /// TBD + /// Checks to see if the current endpoint is writable or not. /// - /// TBD - /// TBD + /// The actor who owns the endpoint. + /// true is writable, false otherwise. public bool IsWritable(IActorRef endpoint) { return _writableToAddress.ContainsKey(endpoint); } /// - /// TBD + /// Checks to see if the current endpoint is read-only or not. /// - /// TBD - /// TBD + /// The actor who owns the endpoint. + /// true is read-only, false otherwise. public bool IsReadOnly(IActorRef endpoint) { return _readonlyToAddress.ContainsKey(endpoint); } /// - /// TBD + /// Checks to see if the specified address is already quarantined or not. /// - /// TBD - /// TBD - /// TBD + /// The address to check. + /// The current UID of . + /// true if this system is under quarantine with its current UID, false otherwise. public bool IsQuarantined(Address address, int uid) { // timeOfRelease is only used for garbage collection. If an address is still probed, we should report the @@ -180,31 +180,31 @@ public bool IsQuarantined(Address address, int uid) } /// - /// TBD + /// Find the "refused" UID for the specified address, if it exists. /// - /// TBD - /// TBD + /// The address to check. + /// The refused UID if one exists for this address; otherwise null. public int? RefuseUid(Address address) { // timeOfRelease is only used for garbage collection. If an address is still probed, we should report the // known fact that it is quarantined. var policy = WritableEndpointWithPolicyFor(address); - var q = policy as EndpointManager.Quarantined; - var p = policy as EndpointManager.Pass; - var g = policy as EndpointManager.Gated; - var w = policy as EndpointManager.WasGated; - if (q != null) return q.Uid; - if (p != null) return p.RefuseUid; - if (g != null) return g.RefuseUid; - if (w != null) return w.RefuseUid; - return null; + switch (policy) + { + case EndpointManager.Quarantined q: + return q.Uid; + default: + if (_addressToRefuseUid.ContainsKey(address)) + return _addressToRefuseUid[address].Item1; + return null; + } } /// - /// TBD + /// Return the writable endpoint policy for the provided remote address, if it exists. /// - /// TBD - /// TBD + /// The remote address to check. + /// The EndpointManager.EndpointPolicy if we have one for this address, null otherwise. public EndpointManager.EndpointPolicy WritableEndpointWithPolicyFor(Address address) { _addressToWritable.TryGetValue(address, out var tmp); @@ -212,22 +212,22 @@ public EndpointManager.EndpointPolicy WritableEndpointWithPolicyFor(Address addr } /// - /// TBD + /// Check if we have a writable endpoint for the provided address. /// - /// TBD - /// TBD - public bool HasWriteableEndpointFor(Address address) + /// The address to check. + /// true if we have a writable + public bool HasWritableEndpointFor(Address address) { var policy = WritableEndpointWithPolicyFor(address); - return policy is EndpointManager.Pass || policy is EndpointManager.WasGated; + return policy is EndpointManager.Pass; } /// /// Marking an endpoint as failed means that we will not try to connect to the remote system within /// the gated period but it is ok for the remote system to try to connect with us (inbound-only.) /// - /// TBD - /// TBD + /// The endpoint actor. + /// The time to release the failure policy. public void MarkAsFailed(IActorRef endpoint, Deadline timeOfRelease) { if (IsWritable(endpoint)) @@ -270,27 +270,28 @@ public void MarkAsFailed(IActorRef endpoint, Deadline timeOfRelease) } /// - /// TBD + /// Mark the current remote address as quarantined. /// - /// TBD - /// TBD - /// TBD + /// The address to quarantine. + /// The UID of the current address. + /// The timeframe to release the quarantine. public void MarkAsQuarantined(Address address, int uid, Deadline timeOfRelease) { _addressToWritable[address] = new EndpointManager.Quarantined(uid, timeOfRelease); + _addressToRefuseUid[address] = Tuple.Create(uid, timeOfRelease); } /// - /// TBD + /// Remove the policy for the current address. /// - /// TBD + /// The address to prune policy settings for. public void RemovePolicy(Address address) { _addressToWritable.Remove(address); } /// - /// TBD + /// The list of all current endpoint actors. /// public IList AllEndpoints { @@ -298,24 +299,25 @@ public IList AllEndpoints } /// - /// TBD + /// Prune all old endpoints. /// public void Prune() { - _addressToWritable = _addressToWritable.Where(x => - !(x.Value is EndpointManager.Quarantined - && !((EndpointManager.Quarantined)x.Value).Deadline.HasTimeLeft)).Select(entry => + _addressToWritable = _addressToWritable.Where(x => { - var key = entry.Key; - var policy = entry.Value; - if (policy is EndpointManager.Gated) + switch (x.Value) { - var gated = (EndpointManager.Gated) policy; - if (gated.TimeOfRelease.HasTimeLeft) return entry; - return new KeyValuePair(key,new EndpointManager.WasGated(gated.RefuseUid)); + case EndpointManager.Gated g when g.TimeOfRelease.HasTimeLeft: + case EndpointManager.Quarantined q when q.Deadline.HasTimeLeft: + case EndpointManager.Pass p: + return true; + default: + return false; } - return entry; }).ToDictionary(key => key.Key, value => value.Value); + + _addressToRefuseUid = _addressToRefuseUid.Where(x => x.Value.Item2.HasTimeLeft) + .ToDictionary(x => x.Key, x => x.Value); } /// From 8870fa8d2fde5b789b5a91298b2db918bbec5c86 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 12:00:30 -0500 Subject: [PATCH 11/36] endpoint manager clean-up --- src/core/Akka.Remote/EndpointManager.cs | 44 +++++-------------------- 1 file changed, 9 insertions(+), 35 deletions(-) diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index f8e9915cb7a..1ade56bb02c 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -65,12 +65,12 @@ public Pass(IActorRef endpoint, int? uid) /// /// The actor who owns the current endpoint /// - public IActorRef Endpoint { get; private set; } + public IActorRef Endpoint { get; } /// /// The endpoint UID, if it's currently known /// - public int? Uid { get; private set; } + public int? Uid { get; } } /// @@ -79,21 +79,13 @@ public Pass(IActorRef endpoint, int? uid) /// public sealed class Gated : EndpointPolicy { - /// - /// TBD - /// - /// TBD - /// TBD public Gated(Deadline deadline) : base(true) { TimeOfRelease = deadline; } - /// - /// TBD - /// - public Deadline TimeOfRelease { get; private set; } + public Deadline TimeOfRelease { get; } } /// @@ -101,11 +93,6 @@ public Gated(Deadline deadline) /// public sealed class Quarantined : EndpointPolicy { - /// - /// TBD - /// - /// TBD - /// TBD public Quarantined(int uid, Deadline deadline) : base(true) { @@ -113,15 +100,9 @@ public Quarantined(int uid, Deadline deadline) Deadline = deadline; } - /// - /// TBD - /// - public int Uid { get; private set; } + public int Uid { get; } - /// - /// TBD - /// - public Deadline Deadline { get; private set; } + public Deadline Deadline { get; } } #endregion @@ -436,10 +417,10 @@ public ResendState(int uid, AckedReceiveBuffer buffer) #endregion /// - /// TBD + /// Creates a new instance. /// - /// TBD - /// TBD + /// The HOCON configuration for the current . + /// The "remoting" logging source. public EndpointManager(Config config, ILoggingAdapter log) { _conf = config; @@ -474,10 +455,7 @@ public EndpointManager(Config config, ILoggingAdapter log) private readonly ConcurrentDictionary _receiveBuffers = new ConcurrentDictionary(); - private bool RetryGateEnabled - { - get { return _settings.RetryGateClosedFor > TimeSpan.Zero; } - } + private bool RetryGateEnabled => _settings.RetryGateClosedFor > TimeSpan.Zero; private TimeSpan PruneInterval { @@ -538,10 +516,6 @@ private void KeepQuarantinedOr(Address remoteAddress, Action body) #region ActorBase overrides - /// - /// TBD - /// - /// TBD protected override SupervisorStrategy SupervisorStrategy() { return new OneForOneStrategy(ex => From dcfe3b205fd76309815098a99a2d19c29b315553 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 12:22:20 -0500 Subject: [PATCH 12/36] finished cleaning up EndpointRegistry --- src/core/Akka.Remote/EndpointManager.cs | 84 +++++++++++------------- src/core/Akka.Remote/EndpointRegistry.cs | 11 +--- 2 files changed, 39 insertions(+), 56 deletions(-) diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index 1ade56bb02c..a9fe239840b 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -520,11 +520,32 @@ protected override SupervisorStrategy SupervisorStrategy() { return new OneForOneStrategy(ex => { - var directive = Directive.Stop; - - ex.Match() - .With(ia => + Directive Hopeless(HopelessAssociation e) + { + switch (e) { + case HopelessAssociation h when h.Uid != null: + _log.Error(e.InnerException ?? e, "Association to [{0}] with UID [{1}] is irrecoverably failed. Quarantining address.", h.RemoteAddress, h.Uid); + if (_settings.QuarantineDuration.HasValue && _settings.QuarantineDuration != TimeSpan.MaxValue) + { + // have a finite quarantine duration specified in settings. + // If we don't have one specified, don't bother quarantining - it's disabled. + _endpoints.MarkAsQuarantined(h.RemoteAddress, h.Uid.Value, Deadline.Now + _settings.QuarantineDuration); + _eventPublisher.NotifyListeners(new QuarantinedEvent(h.RemoteAddress, h.Uid.Value)); + } + + return Directive.Stop; + default: // no UID found + _log.Warning("Association to [{0}] with unknown UID is irrecoverably failed. Address cannot be quarantined without knowing the UID, gating instead for {1} ms.", + e.RemoteAddress, _settings.RetryGateClosedFor.TotalMilliseconds); + _endpoints.MarkAsFailed(Sender, Deadline.Now + _settings.RetryGateClosedFor); + return Directive.Stop; + } + } + + switch (ex) + { + case InvalidAssociation ia: KeepQuarantinedOr(ia.RemoteAddress, () => { var causedBy = ia.InnerException == null @@ -537,56 +558,28 @@ protected override SupervisorStrategy SupervisorStrategy() if (ia.DisassociationInfo.HasValue && ia.DisassociationInfo == DisassociateInfo.Quarantined) Context.System.EventStream.Publish(new ThisActorSystemQuarantinedEvent(ia.LocalAddress, ia.RemoteAddress)); - - directive = Directive.Stop; - }) - .With(shutdown => - { + return Directive.Stop; + case ShutDownAssociation shutdown: KeepQuarantinedOr(shutdown.RemoteAddress, () => { _log.Debug("Remote system with address [{0}] has shut down. Address is now gated for {1}ms, all messages to this address will be delivered to dead letters.", - shutdown.RemoteAddress, _settings.RetryGateClosedFor.TotalMilliseconds); + shutdown.RemoteAddress, _settings.RetryGateClosedFor.TotalMilliseconds); _endpoints.MarkAsFailed(Sender, Deadline.Now + _settings.RetryGateClosedFor); }); - directive = Directive.Stop; - }) - .With(hopeless => - { - if (hopeless.Uid.HasValue) - { - _log.Error(hopeless.InnerException, "Association to [{0}] with UID [{1}] is irrecoverably failed. Quarantining address.", - hopeless.RemoteAddress, hopeless.Uid); - if (_settings.QuarantineDuration.HasValue) - { - _endpoints.MarkAsQuarantined(hopeless.RemoteAddress, hopeless.Uid.Value, - Deadline.Now + _settings.QuarantineDuration.Value); - _eventPublisher.NotifyListeners(new QuarantinedEvent(hopeless.RemoteAddress, - hopeless.Uid.Value)); - } - } - else - { - _log.Warning("Association to [{0}] with unknown UID is irrecoverably failed. Address cannot be quarantined without knowing the UID, gating instead for {1} ms.", - hopeless.RemoteAddress, _settings.RetryGateClosedFor.TotalMilliseconds); - _endpoints.MarkAsFailed(Sender, Deadline.Now + _settings.RetryGateClosedFor); - } - directive = Directive.Stop; - }) - .Default(msg => - { - if (msg is EndpointDisassociatedException || msg is EndpointAssociationException) { } //no logging + return Directive.Stop; + case HopelessAssociation h: + return Hopeless(h); + case ActorInitializationException i when i.InnerException is HopelessAssociation h2: + return Hopeless(h2); + default: + if (ex is EndpointDisassociatedException || ex is EndpointAssociationException) { } //no logging else { _log.Error(ex, ex.Message); } _endpoints.MarkAsFailed(Sender, Deadline.Now + _settings.RetryGateClosedFor); - directive = Directive.Stop; - }); - - return directive; + return Directive.Stop; + } }, false); } - /// - /// TBD - /// protected override void PreStart() { if (PruneTimerCancelleable != null) @@ -594,9 +587,6 @@ protected override void PreStart() base.PreStart(); } - /// - /// TBD - /// protected override void PostStop() { if (PruneTimerCancelleable != null) diff --git a/src/core/Akka.Remote/EndpointRegistry.cs b/src/core/Akka.Remote/EndpointRegistry.cs index ad29e4273df..7b30e88fad5 100644 --- a/src/core/Akka.Remote/EndpointRegistry.cs +++ b/src/core/Akka.Remote/EndpointRegistry.cs @@ -241,14 +241,7 @@ public void MarkAsFailed(IActorRef endpoint, Deadline timeOfRelease) } else if (policy is EndpointManager.Pass) { - _addressToWritable[address] = new EndpointManager.Gated(timeOfRelease, - policy.AsInstanceOf().RefuseUid); - _writableToAddress.Remove(endpoint); - } - else if (policy is EndpointManager.WasGated) - { - _addressToWritable[address] = new EndpointManager.Gated(timeOfRelease, - policy.AsInstanceOf().RefuseUid); + _addressToWritable[address] = new EndpointManager.Gated(timeOfRelease); _writableToAddress.Remove(endpoint); } else if (policy is EndpointManager.Gated) @@ -258,7 +251,7 @@ public void MarkAsFailed(IActorRef endpoint, Deadline timeOfRelease) } else { - _addressToWritable[address] = new EndpointManager.Gated(timeOfRelease, null); + _addressToWritable[address] = new EndpointManager.Gated(timeOfRelease); _writableToAddress.Remove(endpoint); } } From 357feb805a2a2db5c17639d71dc14c0d9994b2db Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 12:33:59 -0500 Subject: [PATCH 13/36] more EndpointManager cleanup --- src/core/Akka.Remote/EndpointManager.cs | 104 +++++++++-------------- src/core/Akka.Remote/EndpointRegistry.cs | 15 ---- 2 files changed, 40 insertions(+), 79 deletions(-) diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index a9fe239840b..a8e76ea2535 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -641,8 +641,7 @@ into g if (x.transports.Count > 1) { throw new RemoteTransportException( - string.Format("There are more than one transports listening on local address {0}", - x.address)); + $"There are more than one transports listening on local address {x.address}"); } return new KeyValuePair(x.address, x.transports.Head().Item1.ProtocolTransport); @@ -719,21 +718,16 @@ protected void Accepting() Context.Stop(p.Endpoint); } // or it does not match with the UID to be quarantined - else if (!uidOption.HasValue && p.RefuseUid != quarantineUid) + else if (!uidOption.HasValue && _endpoints.RefuseUid(quarantine.RemoteAddress) != quarantineUid) { // the quarantine uid may be got fresh by cluster gossip, so update refuseUid for late handle when the writer got uid - _endpoints.RegisterWritableEndpointRefuseUid(quarantine.RemoteAddress, quarantineUid.Value); + _endpoints.RegisterWritableEndpointRefuseUid(quarantine.RemoteAddress, quarantineUid.Value, Deadline.Now + _settings.QuarantineDuration); } else { //the quarantine uid has lost the race with some failure, do nothing } } - else if (policy.Item1 is WasGated wg && policy.Item2 != null) - { - if (wg.RefuseUid == policy.Item2) - _endpoints.RegisterWritableEndpointRefuseUid(quarantine.RemoteAddress, policy.Item2.Value); - } else if (policy.Item1 is Quarantined && policy.Item2 != null && policy.Item1.AsInstanceOf().Uid == policy.Item2.Value) { // the UID to be quarantined already exists, do nothing @@ -795,31 +789,29 @@ bool MatchesQuarantine(AkkaProtocolHandle handle) Receive(send => { var recipientAddress = send.Recipient.Path.Address; - IActorRef CreateAndRegisterWritingEndpoint(int? refuseUid) => _endpoints.RegisterWritableEndpoint(recipientAddress, CreateEndpoint(recipientAddress, send.Recipient.LocalAddressToUse, _transportMapping[send.Recipient.LocalAddressToUse], _settings, writing: true, handleOption: null, refuseUid: refuseUid), uid: null, refuseUid: refuseUid); + IActorRef CreateAndRegisterWritingEndpoint() => _endpoints.RegisterWritableEndpoint(recipientAddress, + CreateEndpoint(recipientAddress, send.Recipient.LocalAddressToUse, _transportMapping[send.Recipient.LocalAddressToUse], + _settings, writing: true, handleOption: null), uid: null); // pattern match won't throw a NullReferenceException if one is returned by WritableEndpointWithPolicyFor - _endpoints.WritableEndpointWithPolicyFor(recipientAddress).Match() - .With( - pass => - { - pass.Endpoint.Tell(send); - }) - .With(gated => - { - if (gated.TimeOfRelease.IsOverdue) CreateAndRegisterWritingEndpoint(gated.RefuseUid).Tell(send); + switch (_endpoints.WritableEndpointWithPolicyFor(recipientAddress)) + { + case Pass pass: + pass.Endpoint.Tell(send); + break; + case Gated gated: + if (gated.TimeOfRelease.IsOverdue) CreateAndRegisterWritingEndpoint().Tell(send); else Context.System.DeadLetters.Tell(send); - }) - .With(wasGated => - { - CreateAndRegisterWritingEndpoint(wasGated.RefuseUid).Tell(send); - }) - .With(quarantined => - { + break; + case Quarantined quarantined: // timeOfRelease is only used for garbage collection reasons, therefore it is ignored here. We still have // the Quarantined tombstone and we know what UID we don't want to accept, so use it. - CreateAndRegisterWritingEndpoint(quarantined.Uid).Tell(send); - }) - .Default(msg => CreateAndRegisterWritingEndpoint(null).Tell(send)); + CreateAndRegisterWritingEndpoint().Tell(send); + break; + default: + CreateAndRegisterWritingEndpoint().Tell(send); + break; + } }); Receive(ia => HandleInboundAssociation(ia, false)); Receive(endpoint => AcceptPendingReader(endpoint.Writer)); @@ -832,44 +824,28 @@ bool MatchesQuarantine(AkkaProtocolHandle handle) Receive(tookover => RemovePendingReader(tookover.Writer, tookover.ProtocolHandle)); Receive(gotuid => { - + var refuseUidOption = _endpoints.RefuseUid(gotuid.RemoteAddress); var policy = _endpoints.WritableEndpointWithPolicyFor(gotuid.RemoteAddress); - var pass = policy as Pass; - if (pass != null) - { - if (pass.RefuseUid == gotuid.Uid) - { - _endpoints.MarkAsQuarantined(gotuid.RemoteAddress, gotuid.Uid, - Deadline.Now + _settings.QuarantineDuration); - _eventPublisher.NotifyListeners(new QuarantinedEvent(gotuid.RemoteAddress, gotuid.Uid)); - Context.Stop(pass.Endpoint); - } - else - { - _endpoints.RegisterWritableEndpointUid(gotuid.RemoteAddress, gotuid.Uid); - } - HandleStashedInbound(Sender, writerIsIdle: false); - } - else if (policy is WasGated) + switch (policy) { - var wg = (WasGated) policy; - if (wg.RefuseUid == gotuid.Uid) - { - _endpoints.MarkAsQuarantined(gotuid.RemoteAddress, gotuid.Uid, - Deadline.Now + _settings.QuarantineDuration); - _eventPublisher.NotifyListeners(new QuarantinedEvent(gotuid.RemoteAddress, gotuid.Uid)); - } - else - { - _endpoints.RegisterWritableEndpointUid(gotuid.RemoteAddress, gotuid.Uid); - } - HandleStashedInbound(Sender, writerIsIdle: false); - } - else - { - // the GotUid might have lost the race with some failure + case Pass pass: + if (refuseUidOption == gotuid.Uid) + { + _endpoints.MarkAsQuarantined(gotuid.RemoteAddress, gotuid.Uid, + Deadline.Now + _settings.QuarantineDuration); + _eventPublisher.NotifyListeners(new QuarantinedEvent(gotuid.RemoteAddress, gotuid.Uid)); + Context.Stop(pass.Endpoint); + } + else + { + _endpoints.RegisterWritableEndpointUid(gotuid.RemoteAddress, gotuid.Uid); + } + HandleStashedInbound(Sender, writerIsIdle: false); + break; + default: + // the GotUid might have lost the race with some failure + break; } - }); Receive(idle => { diff --git a/src/core/Akka.Remote/EndpointRegistry.cs b/src/core/Akka.Remote/EndpointRegistry.cs index 7b30e88fad5..8dbc93ae510 100644 --- a/src/core/Akka.Remote/EndpointRegistry.cs +++ b/src/core/Akka.Remote/EndpointRegistry.cs @@ -312,21 +312,6 @@ public void Prune() _addressToRefuseUid = _addressToRefuseUid.Where(x => x.Value.Item2.HasTimeLeft) .ToDictionary(x => x.Key, x => x.Value); } - - /// - /// Internal function used for filtering endpoints that need to be pruned due to non-recovery past their deadlines - /// - private static bool PruneFilterFunction(EndpointManager.EndpointPolicy policy) - { - var rValue = true; - - policy.Match() - .With(g => rValue = g.TimeOfRelease.HasTimeLeft) - .With(q => rValue = q.Deadline.HasTimeLeft) - .Default(msg => rValue = true); - - return rValue; - } } } From 4cf89d383b7e328de30c9390b2e3556a04cd2ba3 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 12:41:57 -0500 Subject: [PATCH 14/36] modified AssociationHandle to allow explicit DEBUG logging of disassociation reasons --- src/core/Akka.Remote/Transport/Transport.cs | 26 ++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/core/Akka.Remote/Transport/Transport.cs b/src/core/Akka.Remote/Transport/Transport.cs index 4422e96d4db..8143227422c 100644 --- a/src/core/Akka.Remote/Transport/Transport.cs +++ b/src/core/Akka.Remote/Transport/Transport.cs @@ -333,10 +333,10 @@ public void Notify(IAssociationEvent ev) public abstract class AssociationHandle { /// - /// TBD + /// Creates a handle to an association between two remote addresses. /// - /// TBD - /// TBD + /// The local address to use. + /// The remote address to use. protected AssociationHandle(Address localAddress, Address remoteAddress) { LocalAddress = localAddress; @@ -384,8 +384,28 @@ protected AssociationHandle(Address localAddress, Address remoteAddress) /// /// The transport that provides the handle MUST guarantee that could be called arbitrarily many times. /// + [Obsolete("Use the method that states reasons to make sure disassociation reasons are logged.")] public abstract void Disassociate(); + /// + /// Closes the underlying transport link, if needed. Some transports might not need an explicit teardown (UDP) and some + /// transports may not support it. Remote endpoint of the channel or connection MAY be notified, but this is not + /// guaranteed. + /// + /// The transport that provides the handle MUST guarantee that could be called arbitrarily many times. + /// + public void Disassociate(string reason, ILoggingAdapter log) + { + if (log.IsDebugEnabled) + { + log.Debug("Association between local [{0}] and remote [{1}] was disassociated because {2}", LocalAddress, RemoteAddress, reason); + } + +#pragma warning disable 618 + Disassociate(); +#pragma warning restore 618 + } + /// public override bool Equals(object obj) { From 6febaff63a730ef17a976e90e43ab20059ee13a7 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 13:05:41 -0500 Subject: [PATCH 15/36] final pass of EndpointManager updates from https://github.com/akka/akka/pull/23617 --- src/core/Akka.Remote/EndpointManager.cs | 100 ++++++++++++------------ 1 file changed, 52 insertions(+), 48 deletions(-) diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index a8e76ea2535..2908bd172ed 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -919,15 +919,16 @@ private void HandleInboundAssociation(InboundAssociation ia, bool writerIsIdle) { var endpoint = readonlyEndpoint.Item1; if (_pendingReadHandoffs.TryGetValue(endpoint, out var protocolHandle)) - protocolHandle.Disassociate(); + protocolHandle.Disassociate("the existing readOnly association was replaced by a new incoming one", _log); - _pendingReadHandoffs.AddOrSet(endpoint, handle); + _pendingReadHandoffs[endpoint] = handle; endpoint.Tell(new EndpointWriter.TakeOver(handle, Self)); - _endpoints.WritableEndpointWithPolicyFor(handle.RemoteAddress).Match() - .With(pass => - { + switch (_endpoints.WritableEndpointWithPolicyFor(handle.RemoteAddress)) + { + case Pass pass: pass.Endpoint.Tell(new ReliableDeliverySupervisor.Ungate()); - }); + break; + } } else { @@ -936,41 +937,46 @@ private void HandleInboundAssociation(InboundAssociation ia, bool writerIsIdle) else { var policy = _endpoints.WritableEndpointWithPolicyFor(handle.RemoteAddress); - var pass = policy as Pass; - if (pass != null && !pass.Uid.HasValue) + switch (policy) { - // Idle writer will never send a GotUid or a Terminated so we need to "provoke it" - // to get an unstash event - if (!writerIsIdle) - { - pass.Endpoint.Tell(ReliableDeliverySupervisor.IsIdle.Instance); - var stashedInboundForEp = _stashedInbound.GetOrElse(pass.Endpoint, - new List()); - stashedInboundForEp.Add(ia); - _stashedInbound[pass.Endpoint] = stashedInboundForEp; - } - else - CreateAndRegisterEndpoint(handle, _endpoints.RefuseUid(handle.RemoteAddress)); - } - else if (pass != null) // has a UID value - { - if (handle.HandshakeInfo.Uid == pass.Uid) - { - _pendingReadHandoffs.GetOrElse(pass.Endpoint, null)?.Disassociate(); - _pendingReadHandoffs.AddOrSet(pass.Endpoint, handle); - pass.Endpoint.Tell(new EndpointWriter.StopReading(pass.Endpoint, Self)); - pass.Endpoint.Tell(new ReliableDeliverySupervisor.Ungate()); - } - else - { - Context.Stop(pass.Endpoint); - _endpoints.UnregisterEndpoint(pass.Endpoint); - _pendingReadHandoffs.Remove(pass.Endpoint); - CreateAndRegisterEndpoint(handle, pass.Uid); - } + case Pass pass when !pass.Uid.HasValue: // pass, but UID is unknown + // Idle writer will never send a GotUid or a Terminated so we need to "provoke it" + // to get an unstash event + if (!writerIsIdle) + { + pass.Endpoint.Tell(ReliableDeliverySupervisor.IsIdle.Instance); + + // BUG: does this wipe out existing stashed inbound connections? Need to look up Scala getOrElse behavior + var stashedInboundForEp = _stashedInbound.GetOrElse(pass.Endpoint, + new List()); + stashedInboundForEp.Add(ia); + _stashedInbound[pass.Endpoint] = stashedInboundForEp; + } + else + CreateAndRegisterEndpoint(handle); + break; + case Pass pWithUid: // pass with known UID + if (handle.HandshakeInfo.Uid == pWithUid.Uid) + { + _pendingReadHandoffs.GetOrElse(pWithUid.Endpoint, null)?.Disassociate("the existing writable association was replaced by a new incoming one", _log); + _pendingReadHandoffs[pWithUid.Endpoint] = handle; + pWithUid.Endpoint.Tell(new EndpointWriter.StopReading(pWithUid.Endpoint, Self)); + pWithUid.Endpoint.Tell(new ReliableDeliverySupervisor.Ungate()); + } + else + { + Context.Stop(pWithUid.Endpoint); + _endpoints.UnregisterEndpoint(pWithUid.Endpoint); + _pendingReadHandoffs.Remove(pWithUid.Endpoint); + _endpoints.MarkAsQuarantined(handle.RemoteAddress, pWithUid.Uid.Value, Deadline.Now + _settings.QuarantineDuration); + CreateAndRegisterEndpoint(handle); + } + + break; + default: + CreateAndRegisterEndpoint(handle); + break; } - else - CreateAndRegisterEndpoint(handle, _endpoints.RefuseUid(handle.RemoteAddress)); } } } @@ -1072,7 +1078,7 @@ private void RemovePendingReader(IActorRef takingOverFrom, AkkaProtocolHandle wi _pendingReadHandoffs.Remove(takingOverFrom); } - private void CreateAndRegisterEndpoint(AkkaProtocolHandle handle, int? refuseUid) + private void CreateAndRegisterEndpoint(AkkaProtocolHandle handle) { var writing = _settings.UsePassiveConnections && !_endpoints.HasWritableEndpointFor(handle.RemoteAddress); _eventPublisher.NotifyListeners(new AssociatedEvent(handle.LocalAddress, handle.RemoteAddress, true)); @@ -1082,12 +1088,11 @@ private void CreateAndRegisterEndpoint(AkkaProtocolHandle handle, int? refuseUid _transportMapping[handle.LocalAddress], _settings, writing, - handle, - refuseUid); + handle); if (writing) { - _endpoints.RegisterWritableEndpoint(handle.RemoteAddress, endpoint, handle.HandshakeInfo.Uid, refuseUid); + _endpoints.RegisterWritableEndpoint(handle.RemoteAddress, endpoint, handle.HandshakeInfo.Uid); } else { @@ -1103,12 +1108,12 @@ private IActorRef CreateEndpoint( AkkaProtocolTransport transport, RemoteSettings endpointSettings, bool writing, - AkkaProtocolHandle handleOption = null, - int? refuseUid = null) + AkkaProtocolHandle handleOption = null) { System.Diagnostics.Debug.Assert(_transportMapping.ContainsKey(localAddress)); // refuseUid is ignored for read-only endpoints since the UID of the remote system is already known and has passed // quarantine checks + var refuseUid = _endpoints.RefuseUid(remoteAddress); IActorRef endpointActor; @@ -1121,8 +1126,7 @@ private IActorRef CreateEndpoint( remoteAddress, refuseUid, transport, endpointSettings, new AkkaPduProtobuffCodec(Context.System), _receiveBuffers, endpointSettings.Dispatcher) .WithDeploy(Deploy.Local)), - string.Format("reliableEndpointWriter-{0}-{1}", AddressUrlEncoder.Encode(remoteAddress), - _endpointId.Next())); + $"reliableEndpointWriter-{AddressUrlEncoder.Encode(remoteAddress)}-{_endpointId.Next()}"); } else { @@ -1133,7 +1137,7 @@ private IActorRef CreateEndpoint( transport, endpointSettings, new AkkaPduProtobuffCodec(Context.System), _receiveBuffers, reliableDeliverySupervisor: null) .WithDeploy(Deploy.Local)), - string.Format("endpointWriter-{0}-{1}", AddressUrlEncoder.Encode(remoteAddress), _endpointId.Next())); + $"endpointWriter-{AddressUrlEncoder.Encode(remoteAddress)}-{_endpointId.Next()}"); } Context.Watch(endpointActor); From 2155edcc9cbbbd700ef6771a5f69398293f62ec0 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 13:14:38 -0500 Subject: [PATCH 16/36] fixed EndpointRegistrySpec --- .../Akka.Remote.Tests/EndpointRegistrySpec.cs | 31 +++++++++---------- src/core/Akka.Remote/EndpointManager.cs | 2 +- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs b/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs index ec205d5ed10..ebf699dd710 100644 --- a/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs +++ b/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs @@ -29,12 +29,12 @@ public EndpointRegistrySpec() } [Fact] - public void EndpointRegistry_must_be_able_to_register_a_writeable_endpoint_and_policy() + public void EndpointRegistry_must_be_able_to_register_a_writable_endpoint_and_policy() { var reg = new EndpointRegistry(); Assert.Null(reg.WritableEndpointWithPolicyFor(address1)); - Assert.Equal(actorA, reg.RegisterWritableEndpoint(address1, actorA,null,null)); + Assert.Equal(actorA, reg.RegisterWritableEndpoint(address1, actorA,null)); Assert.IsType(reg.WritableEndpointWithPolicyFor(address1)); Assert.Equal(actorA, reg.WritableEndpointWithPolicyFor(address1).AsInstanceOf().Endpoint); @@ -68,7 +68,7 @@ public void EndpointRegistry_must_be_able_to_register_writable_and_readonly_endp Assert.Null(reg.WritableEndpointWithPolicyFor(address1)); Assert.Equal(actorA, reg.RegisterReadOnlyEndpoint(address1, actorA, 1)); - Assert.Equal(actorB, reg.RegisterWritableEndpoint(address1, actorB, null,null)); + Assert.Equal(actorB, reg.RegisterWritableEndpoint(address1, actorB, null)); Assert.Equal(Tuple.Create(actorA,1), reg.ReadOnlyEndpointFor(address1)); Assert.Equal(actorB, reg.WritableEndpointWithPolicyFor(address1).AsInstanceOf().Endpoint); @@ -85,11 +85,10 @@ public void EndpointRegistry_must_be_able_to_register_Gated_policy_for_an_addres { var reg = new EndpointRegistry(); Assert.Null(reg.WritableEndpointWithPolicyFor(address1)); - reg.RegisterWritableEndpoint(address1, actorA, null, null); + reg.RegisterWritableEndpoint(address1, actorA, null); var deadline = Deadline.Now; reg.MarkAsFailed(actorA, deadline); Assert.Equal(deadline, reg.WritableEndpointWithPolicyFor(address1).AsInstanceOf().TimeOfRelease); - Assert.Null(reg.WritableEndpointWithPolicyFor(address1).AsInstanceOf().RefuseUid); Assert.False(reg.IsReadOnly(actorA)); Assert.False(reg.IsWritable(actorA)); } @@ -107,8 +106,8 @@ public void EndpointRegistry_must_remove_readonly_endpoints_if_marked_as_failed( public void EndpointRegistry_must_keep_tombstones_when_removing_an_endpoint() { var reg = new EndpointRegistry(); - reg.RegisterWritableEndpoint(address1, actorA, null, null); - reg.RegisterWritableEndpoint(address2, actorB, null, null); + reg.RegisterWritableEndpoint(address1, actorA, null); + reg.RegisterWritableEndpoint(address2, actorB, null); var deadline = Deadline.Now; reg.MarkAsFailed(actorA, deadline); reg.MarkAsQuarantined(address2, 42, deadline); @@ -117,7 +116,6 @@ public void EndpointRegistry_must_keep_tombstones_when_removing_an_endpoint() reg.UnregisterEndpoint(actorB); Assert.Equal(deadline, reg.WritableEndpointWithPolicyFor(address1).AsInstanceOf().TimeOfRelease); - Assert.Null(reg.WritableEndpointWithPolicyFor(address1).AsInstanceOf().RefuseUid); Assert.Equal(deadline, reg.WritableEndpointWithPolicyFor(address2).AsInstanceOf().Deadline); Assert.Equal(42, reg.WritableEndpointWithPolicyFor(address2).AsInstanceOf().Uid); } @@ -126,14 +124,13 @@ public void EndpointRegistry_must_keep_tombstones_when_removing_an_endpoint() public void EndpointRegistry_should_prune_outdated_Gated_directives_properly() { var reg = new EndpointRegistry(); - reg.RegisterWritableEndpoint(address1, actorA, null, null); - reg.RegisterWritableEndpoint(address2, actorB, null, null); + reg.RegisterWritableEndpoint(address1, actorA, null); + reg.RegisterWritableEndpoint(address2, actorB, null); reg.MarkAsFailed(actorA, Deadline.Now); var farIntheFuture = Deadline.Now + TimeSpan.FromSeconds(60); reg.MarkAsFailed(actorB, farIntheFuture); reg.Prune(); - Assert.Null(reg.WritableEndpointWithPolicyFor(address1).AsInstanceOf().RefuseUid); Assert.Equal(farIntheFuture, reg.WritableEndpointWithPolicyFor(address2).AsInstanceOf().TimeOfRelease); } @@ -176,7 +173,7 @@ public void EndpointRegistry_should_overwrite_Quarantine_policy_with_Pass_on_Reg Assert.True(reg.IsQuarantined(address1, quarantinedUid)); var writableUid = 43; - reg.RegisterWritableEndpoint(address1, TestActor, writableUid, quarantinedUid); + reg.RegisterWritableEndpoint(address1, TestActor, writableUid); Assert.True(reg.IsWritable(TestActor)); } @@ -187,30 +184,30 @@ public void EndpointRegistry_should_overwrite_Gated_policy_with_Pass_on_Register var deadline = Deadline.Now + TimeSpan.FromMinutes(30); var willBeGated = 42; - reg.RegisterWritableEndpoint(address1, TestActor, willBeGated, null); + reg.RegisterWritableEndpoint(address1, TestActor, willBeGated); Assert.NotNull(reg.WritableEndpointWithPolicyFor(address1)); Assert.True(reg.IsWritable(TestActor)); reg.MarkAsFailed(TestActor, deadline); Assert.False(reg.IsWritable(TestActor)); var writableUid = 43; - reg.RegisterWritableEndpoint(address1, TestActor, writableUid, willBeGated); + reg.RegisterWritableEndpoint(address1, TestActor, writableUid); Assert.True(reg.IsWritable(TestActor)); } [Fact] - public void EndpointRegister_should_not_report_endpoint_as_writeable_if_no_Pass_policy() + public void EndpointRegister_should_not_report_endpoint_as_writable_if_no_Pass_policy() { var reg = new EndpointRegistry(); var deadline = Deadline.Now + TimeSpan.FromMinutes(30); Assert.False(reg.IsWritable(TestActor)); // no policy - reg.RegisterWritableEndpoint(address1, TestActor, 42, null); + reg.RegisterWritableEndpoint(address1, TestActor, 42); Assert.True(reg.IsWritable(TestActor)); // pass reg.MarkAsFailed(TestActor, deadline); Assert.False(reg.IsWritable(TestActor)); // Gated - reg.RegisterWritableEndpoint(address1, TestActor, 43, 42); // restarted + reg.RegisterWritableEndpoint(address1, TestActor, 43); // restarted Assert.True(reg.IsWritable(TestActor)); // pass reg.MarkAsQuarantined(address1, 43, deadline); Assert.False(reg.HasWritableEndpointFor(address1)); // Quarantined diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index 2908bd172ed..e2f77b06be4 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -1067,7 +1067,7 @@ private void AcceptPendingReader(IActorRef takingOverFrom) _pendingReadHandoffs.Remove(takingOverFrom); _eventPublisher.NotifyListeners(new AssociatedEvent(handle.LocalAddress, handle.RemoteAddress, inbound: true)); var endpoint = CreateEndpoint(handle.RemoteAddress, handle.LocalAddress, - _transportMapping[handle.LocalAddress], _settings, false, handle, refuseUid: null); + _transportMapping[handle.LocalAddress], _settings, false, handle); _endpoints.RegisterReadOnlyEndpoint(handle.RemoteAddress, endpoint, handle.HandshakeInfo.Uid); } } From a625bf042c1e6aafcbd0fd3c1ac0790bd04ee096 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 13:29:51 -0500 Subject: [PATCH 17/36] fixed issue with EndpointRegistry quarantine management --- .../Akka.Remote.Tests/EndpointRegistrySpec.cs | 34 ++++++++++++++++++- src/core/Akka.Remote/EndpointRegistry.cs | 13 ++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs b/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs index ebf699dd710..b82782267e0 100644 --- a/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs +++ b/src/core/Akka.Remote.Tests/EndpointRegistrySpec.cs @@ -9,6 +9,7 @@ using Akka.Actor; using Akka.TestKit; using Akka.Util.Internal; +using FluentAssertions; using Xunit; namespace Akka.Remote.Tests @@ -196,7 +197,7 @@ public void EndpointRegistry_should_overwrite_Gated_policy_with_Pass_on_Register } [Fact] - public void EndpointRegister_should_not_report_endpoint_as_writable_if_no_Pass_policy() + public void EndpointRegistry_should_not_report_endpoint_as_writable_if_no_Pass_policy() { var reg = new EndpointRegistry(); var deadline = Deadline.Now + TimeSpan.FromMinutes(30); @@ -212,6 +213,37 @@ public void EndpointRegister_should_not_report_endpoint_as_writable_if_no_Pass_p reg.MarkAsQuarantined(address1, 43, deadline); Assert.False(reg.HasWritableEndpointFor(address1)); // Quarantined } + + [Fact] + public void EndpointRegistry_should_keep_refuseUid_after_register_new_Endpoint() + { + var reg = new EndpointRegistry(); + var deadline = Deadline.Now + TimeSpan.FromMinutes(30); + + reg.RegisterWritableEndpoint(address1, actorA, null); + reg.MarkAsQuarantined(address1, 42, deadline); + reg.RefuseUid(address1).Should().Be(42); + reg.IsQuarantined(address1, 42).Should().BeTrue(); + + reg.UnregisterEndpoint(actorA); + // Quarantined marker is kept so far + var policy = reg.WritableEndpointWithPolicyFor(address1); + policy.Should().BeOfType(); + policy.AsInstanceOf().Uid.Should().Be(42); + policy.AsInstanceOf().Deadline.Should().Be(deadline); + + reg.RefuseUid(address1).Should().Be(42); + reg.IsQuarantined(address1, 42).Should().BeTrue(); + + reg.RegisterWritableEndpoint(address1, actorB, null); + // Quarantined marker is gone + var policy2 = reg.WritableEndpointWithPolicyFor(address1); + policy2.Should().BeOfType(); + policy2.AsInstanceOf().Endpoint.Should().Be(actorB); + // but we still have the refuseUid + reg.RefuseUid(address1).Should().Be(42); + reg.IsQuarantined(address1, 42).Should().BeTrue(); + } } } diff --git a/src/core/Akka.Remote/EndpointRegistry.cs b/src/core/Akka.Remote/EndpointRegistry.cs index 8dbc93ae510..d47f5a2d417 100644 --- a/src/core/Akka.Remote/EndpointRegistry.cs +++ b/src/core/Akka.Remote/EndpointRegistry.cs @@ -176,7 +176,18 @@ public bool IsQuarantined(Address address, int uid) // timeOfRelease is only used for garbage collection. If an address is still probed, we should report the // known fact that it is quarantined. var policy = WritableEndpointWithPolicyFor(address) as EndpointManager.Quarantined; - return policy?.Uid == uid; + switch (policy) + { + case EndpointManager.Quarantined q when q.Uid == uid: + return true; + default: + if (_addressToRefuseUid.ContainsKey(address)) + { + return _addressToRefuseUid[address].Item1 == uid; + } + + return false; + } } /// From 028aec28cd8e2c322f6d1ed22d2f5dd01b21ca01 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 13:46:42 -0500 Subject: [PATCH 18/36] updated the reliable delivery supervisor --- src/core/Akka.Remote/Endpoint.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/core/Akka.Remote/Endpoint.cs b/src/core/Akka.Remote/Endpoint.cs index 67b3acac454..72061750708 100644 --- a/src/core/Akka.Remote/Endpoint.cs +++ b/src/core/Akka.Remote/Endpoint.cs @@ -132,9 +132,9 @@ public void Dispatch(IInternalActorRef recipient, Address recipientAddress, Seri _log.Debug("operating in UntrustedMode, dropping inbound IPossiblyHarmful message of type {0}", payload.GetType()); } - else if (payload is ISystemMessage) + else if (payload is ISystemMessage systemMessage) { - recipient.SendSystemMessage((ISystemMessage)payload); + recipient.SendSystemMessage(systemMessage); } else { @@ -454,7 +454,13 @@ public ReliableDeliverySupervisor( Reset(); // needs to be called at startup _writer = CreateWriter(); // need to create writer at startup Uid = handleOrActive != null ? (int?)handleOrActive.HandshakeInfo.Uid : null; - UidConfirmed = Uid.HasValue; + UidConfirmed = Uid.HasValue && (Uid != _refuseUid); + + if (Uid.HasValue && Uid == _refuseUid) + throw new HopelessAssociation(localAddress, remoteAddress, Uid, + new InvalidOperationException( + $"The remote system [{remoteAddress}] has a UID [{Uid}] that has been quarantined. Association aborted.")); + Receiving(); _autoResendTimer = Context.System.Scheduler.ScheduleTellRepeatedlyCancelable(_settings.SysResendTimeout, _settings.SysResendTimeout, Self, new AttemptSysMsgRedelivery(), Self); @@ -626,6 +632,7 @@ protected void Receiving() { _writer.Forward(stopped); //forward the request }); + Receive(_ => { }); //ok, not gated } private void GoToIdle() @@ -736,6 +743,7 @@ protected void IdleBehavior() }); Receive(stop => Context.Stop(Self)); Receive(stop => stop.ReplyTo.Tell(new EndpointWriter.StoppedReading(stop.Writer))); + Receive(_ => { }); //ok, not gated } /// From 0529addd68af41ab2941a2c012f46e12ee86623d Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 15:01:44 -0500 Subject: [PATCH 19/36] cleaned up some remote metrics code --- src/core/Akka.Remote/RemoteMetricsExtension.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/Akka.Remote/RemoteMetricsExtension.cs b/src/core/Akka.Remote/RemoteMetricsExtension.cs index 3a322cc0c8f..7f22809c105 100644 --- a/src/core/Akka.Remote/RemoteMetricsExtension.cs +++ b/src/core/Akka.Remote/RemoteMetricsExtension.cs @@ -74,13 +74,13 @@ public void LogPayloadBytes(object msg, long payloadBytes) if (payloadBytes >= _logFrameSizeExceeding) { Type type; - if (msg is ActorSelectionMessage) + if (msg is ActorSelectionMessage message) { - type = ((ActorSelectionMessage) msg).Message.GetType(); + type = message.Message.GetType(); } - else if (msg is RouterEnvelope) + else if (msg is RouterEnvelope envelope) { - type = ((RouterEnvelope) msg).Message.GetType(); + type = envelope.Message.GetType(); } else { From b5eb888469672cb5e288086604e37eae84cf8464 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 15:57:01 -0500 Subject: [PATCH 20/36] tidying up --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 4 +-- src/core/Akka.Remote/EndpointManager.cs | 1 - .../Transport/AkkaProtocolTransport.cs | 26 ++++++++++++------- .../Transport/DotNetty/TcpTransport.cs | 3 +-- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index c28af6a878d..61a7e187b54 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -512,14 +512,14 @@ public void Properly_quarantine_stashed_inbound_connections() var inboundHandleTask = remoteTransport.Associate(rawLocalAddress); inboundHandleTask.Wait(TimeSpan.FromSeconds(3)); var inboundHandle = inboundHandleTask.Result; - inboundHandle.ReadHandlerSource.TrySetResult(new ActorHandleEventListener(inboundHandleProbe)); + inboundHandle.ReadHandlerSource.SetResult(new ActorHandleEventListener(inboundHandleProbe)); AwaitAssert(() => { registry.GetRemoteReadHandlerFor(inboundHandle.AsInstanceOf()).Should().NotBeNull(); }); - var pduCodec = new AkkaPduProtobuffCodec(thisSystem); + var pduCodec = new AkkaPduProtobuffCodec(Sys); var handshakePacket = pduCodec.ConstructAssociate(new HandshakeInfo(remoteAddress, remoteUID)); diff --git a/src/core/Akka.Remote/EndpointManager.cs b/src/core/Akka.Remote/EndpointManager.cs index e2f77b06be4..0fe47c2f547 100644 --- a/src/core/Akka.Remote/EndpointManager.cs +++ b/src/core/Akka.Remote/EndpointManager.cs @@ -946,7 +946,6 @@ private void HandleInboundAssociation(InboundAssociation ia, bool writerIsIdle) { pass.Endpoint.Tell(ReliableDeliverySupervisor.IsIdle.Instance); - // BUG: does this wipe out existing stashed inbound connections? Need to look up Scala getOrElse behavior var stashedInboundForEp = _stashedInbound.GetOrElse(pass.Endpoint, new List()); stashedInboundForEp.Add(ia); diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index aa9d49aaaf2..2ff9e230589 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -203,9 +203,9 @@ public AkkaProtocolManager(Transport wrappedTransport, AkkaProtocolSettings sett _settings = settings; } - private Transport _wrappedTransport; + private readonly Transport _wrappedTransport; - private AkkaProtocolSettings _settings; + private readonly AkkaProtocolSettings _settings; /// /// The does not handle recovery of associations, this task is implemented @@ -229,9 +229,9 @@ protected override SupervisorStrategy SupervisorStrategy() /// TBD protected override void Ready(object message) { - message.Match() - .With(ia => //need to create an Inbound ProtocolStateActor - { + switch (message) + { + case InboundAssociation ia: var handle = ia.Association; var stateActorLocalAddress = LocalAddress; var stateActorAssociationListener = AssociationListener; @@ -244,9 +244,17 @@ protected override void Ready(object message) stateActorSettings, new AkkaPduProtobuffCodec(Context.System), failureDetector)), ActorNameFor(handle.RemoteAddress)); - }) - .With(au => CreateOutboundStateActor(au.RemoteAddress, au.StatusPromise, null)) //need to create an Outbound ProtocolStateActor - .With(au => CreateOutboundStateActor(au.RemoteAddress, au.StatusCompletionSource, au.RefuseUid)); + break; + case AssociateUnderlying au: + CreateOutboundStateActor(au.RemoteAddress, au.StatusPromise, null); + break; + case AssociateUnderlyingRefuseUid aur: + CreateOutboundStateActor(aur.RemoteAddress, aur.StatusCompletionSource, aur.RefuseUid); + break; + default: + Unhandled(message); + break; + } } #endregion @@ -278,7 +286,7 @@ private void CreateOutboundStateActor(Address remoteAddress, private FailureDetector CreateTransportFailureDetector() { - return FailureDetectorLoader.LoadFailureDetector(Context, _settings.TransportFailureDetectorImplementationClass, + return Context.LoadFailureDetector(_settings.TransportFailureDetectorImplementationClass, _settings.TransportFailureDetectorConfig); } diff --git a/src/core/Akka.Remote/Transport/DotNetty/TcpTransport.cs b/src/core/Akka.Remote/Transport/DotNetty/TcpTransport.cs index 193ed18c84e..1e5f8c85148 100644 --- a/src/core/Akka.Remote/Transport/DotNetty/TcpTransport.cs +++ b/src/core/Akka.Remote/Transport/DotNetty/TcpTransport.cs @@ -246,8 +246,7 @@ private async Task MapEndpointAsync(EndPoint socketAddress) { IPEndPoint ipEndPoint; - var dns = socketAddress as DnsEndPoint; - if (dns != null) + if (socketAddress is DnsEndPoint dns) ipEndPoint = await DnsToIPEndpoint(dns).ConfigureAwait(false); else ipEndPoint = (IPEndPoint) socketAddress; From 82a70767e2cf365cc6afaa505ce54501b9bf2011 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 17:04:14 -0500 Subject: [PATCH 21/36] added additional spec --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 86 ++++++++++++++++++- src/core/Akka.Remote/Endpoint.cs | 3 +- .../Transport/AkkaProtocolTransport.cs | 22 ++--- 3 files changed, 95 insertions(+), 16 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index 61a7e187b54..dccf4e2941f 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -462,6 +462,88 @@ public async Task Bug_884_Remoting_must_support_reply_to_child_of_Routee() Assert.Equal(reporter, msg.Item2); } + [Fact] + public void Stash_inbound_connections_until_UID_is_known_for_pending_outbound() + { + var localAddress = new Address("akka.test", "system1", "localhost", 1); + var rawLocalAddress = new Address("test", "system1", "localhost", 1); + var remoteAddress = new Address("akka.test", "system2", "localhost", 2); + var rawRemoteAddress = new Address("test", "system2", "localhost", 2); + + var config = ConfigurationFactory.ParseString(@" + akka.remote.enabled-transports = [""akka.remote.test""] + akka.remote.retry-gate-closed-for = 5s + akka.remote.log-remote-lifecycle-events = on + akka.loglevel = DEBUG + + akka.remote.test { + registry-key = TRKAzR + local-address = """ + $"test://{localAddress.System}@{localAddress.Host}:{localAddress.Port}" + @""" + }").WithFallback(_remoteSystem.Settings.Config); + + var thisSystem = ActorSystem.Create("this-system", config); + MuteSystem(thisSystem); + + try + { + // Set up a mock remote system using the test transport + var registry = AssociationRegistry.Get("TRKAzR"); + var remoteTransport = new TestTransport(rawRemoteAddress, registry); + var remoteTransportProbe = CreateTestProbe(); + + registry.RegisterTransport(remoteTransport, Task.FromResult + (new ActorAssociationEventListener(remoteTransportProbe))); + + // Hijack associations through the test transport + AwaitCondition(() => registry.TransportsReady(rawLocalAddress, rawRemoteAddress)); + var testTransport = registry.TransportFor(rawLocalAddress).Item1; + testTransport.WriteBehavior.PushConstant(true); + + // Force an outbound associate on the real system (which we will hijack) + // we send no handshake packet, so this remains a pending connection + var dummySelection = thisSystem.ActorSelection(ActorPath.Parse(remoteAddress + "/user/noonethere")); + dummySelection.Tell("ping", Sys.DeadLetters); + + var remoteHandle = remoteTransportProbe.ExpectMsg(); + remoteHandle.Association.ReadHandlerSource.TrySetResult((IHandleEventListener)(new ActionHandleEventListener(ev => { }))); + + // Now we initiate an emulated inbound connection to the real system + var inboundHandleProbe = CreateTestProbe(); + var inboundHandleTask = remoteTransport.Associate(rawLocalAddress); + inboundHandleTask.Wait(TimeSpan.FromSeconds(3)); + var inboundHandle = inboundHandleTask.Result; + inboundHandle.ReadHandlerSource.SetResult(new ActorHandleEventListener(inboundHandleProbe)); + + AwaitAssert(() => + { + registry.GetRemoteReadHandlerFor(inboundHandle.AsInstanceOf()).Should().NotBeNull(); + }); + + var pduCodec = new AkkaPduProtobuffCodec(Sys); + + var handshakePacket = pduCodec.ConstructAssociate(new HandshakeInfo(remoteAddress, 0)); + var brokenPacket = pduCodec.ConstructPayload(ByteString.CopyFrom(0, 1, 2, 3, 4, 5, 6)); + + // Finish the inbound handshake so now it is handed up to Remoting + inboundHandle.Write(handshakePacket); + // Now bork the connection with a malformed packet that can only signal an error if the Endpoint is already registered + // but not while it is stashed + inboundHandle.Write(brokenPacket); + + // No disassociation now - the connection is still stashed + inboundHandleProbe.ExpectNoMsg(1000); + + // Finish the handshake for the outbound connection - this will unstash the inbound pending connection. + remoteHandle.Association.Write(handshakePacket); + + inboundHandleProbe.ExpectMsg(); + } + finally + { + Shutdown(thisSystem); + } + } + [Fact] public void Properly_quarantine_stashed_inbound_connections() { @@ -474,7 +556,7 @@ public void Properly_quarantine_stashed_inbound_connections() var config = ConfigurationFactory.ParseString(@" akka.remote.enabled-transports = [""akka.remote.test""] akka.remote.retry-gate-closed-for = 5s - akka.remote.log-remote-lifecycle-events = on + akka.remote.log-remote-lifecycle-events = on akka.remote.test { registry-key = JMeMndLLsw @@ -504,7 +586,7 @@ public void Properly_quarantine_stashed_inbound_connections() var dummySelection = thisSystem.ActorSelection(ActorPath.Parse(remoteAddress + "/user/noonethere")); dummySelection.Tell("ping", Sys.DeadLetters); - var remoteHandle = remoteTransportProbe.ExpectMsg(); + var remoteHandle = remoteTransportProbe.ExpectMsg(TimeSpan.FromMinutes(4)); remoteHandle.Association.ReadHandlerSource.TrySetResult((IHandleEventListener)(new ActionHandleEventListener(ev => {}))); // Now we initiate an emulated inbound connection to the real system diff --git a/src/core/Akka.Remote/Endpoint.cs b/src/core/Akka.Remote/Endpoint.cs index 72061750708..afbcfd80b5a 100644 --- a/src/core/Akka.Remote/Endpoint.cs +++ b/src/core/Akka.Remote/Endpoint.cs @@ -1103,7 +1103,8 @@ protected override void PreStart() { if (_handle == null) { - AssociateAsync().PipeTo(Self); + var self = Self; + AssociateAsync().PipeTo(self); } else { diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index 2ff9e230589..0f4744b290e 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -403,42 +403,38 @@ public AkkaProtocolHandle(Address originalLocalAddress, Address originalRemoteAd } /// - /// TBD + /// The current handshake information. /// public readonly HandshakeInfo HandshakeInfo; /// - /// TBD + /// The responsible for this association. /// public readonly IActorRef StateActor; /// - /// TBD + /// The codec instance used for managing transport-specific commands. /// public readonly AkkaPduCodec Codec; - /// - /// TBD - /// - /// TBD - /// TBD + /// public override bool Write(ByteString payload) { return WrappedHandle.Write(Codec.ConstructPayload(payload)); } - /// - /// TBD - /// +#pragma warning disable CS0672 // Member overrides obsolete member + /// public override void Disassociate() +#pragma warning restore CS0672 // Member overrides obsolete member { Disassociate(DisassociateInfo.Unknown); } /// - /// TBD + /// Forces a disassociation of the current transport. /// - /// TBD + /// The reason for disassociating. public void Disassociate(DisassociateInfo info) { StateActor.Tell(new DisassociateUnderlying(info)); From fba4e110c3c8d15c5436ece5354f4497e7cbd8df Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 17:33:16 -0500 Subject: [PATCH 22/36] stashing work --- .../Transport/AkkaProtocolTransport.cs | 99 +++++++++++++++---- 1 file changed, 79 insertions(+), 20 deletions(-) diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index 0f4744b290e..e59d55721ae 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -177,14 +177,14 @@ public async Task Associate(Address remoteAddress, int? refu return (AkkaProtocolHandle)await statusPromise.Task.ConfigureAwait(false); } -#region Static properties + #region Static properties /// /// TBD /// public static AtomicCounter UniqueId = new AtomicCounter(0); -#endregion + #endregion } /// @@ -221,7 +221,7 @@ protected override SupervisorStrategy SupervisorStrategy() return _supervisor; } -#region ActorBase / ActorTransportAdapterManager overrides + #region ActorBase / ActorTransportAdapterManager overrides /// /// TBD @@ -257,9 +257,9 @@ protected override void Ready(object message) } } -#endregion + #endregion -#region Actor creation methods + #region Actor creation methods private string ActorNameFor(Address remoteAddress) { @@ -290,7 +290,7 @@ private FailureDetector CreateTransportFailureDetector() _settings.TransportFailureDetectorConfig); } -#endregion + #endregion } /// @@ -424,7 +424,7 @@ public override bool Write(ByteString payload) } #pragma warning disable CS0672 // Member overrides obsolete member - /// + /// public override void Disassociate() #pragma warning restore CS0672 // Member overrides obsolete member { @@ -492,6 +492,8 @@ internal enum AssociationState /// internal class HeartbeatTimer : INoSerializationVerificationNeeded { } + internal class HandshakeTimer : INoSerializationVerificationNeeded { } + /// /// TBD /// @@ -727,14 +729,16 @@ internal class ForbiddenUidReason { } internal class ProtocolStateActor : FSM { private readonly ILoggingAdapter _log = Context.GetLogger(); - private InitialProtocolStateData _initialData; - private HandshakeInfo _localHandshakeInfo; + public InitialProtocolStateData _initialData; + private readonly HandshakeInfo _localHandshakeInfo; private int? _refuseUid; private AkkaProtocolSettings _settings; private Address _localAddress; private AkkaPduCodec _codec; private FailureDetector _failureDetector; + private const string handshakeTimerKey = "handshake-timer"; + /// /// Constructor for outbound ProtocolStateActors /// @@ -825,12 +829,62 @@ protected ProtocolStateActor(InitialProtocolStateData initialData, HandshakeInfo InitializeFSM(); } -#region FSM bindings + #region FSM bindings private void InitializeFSM() { When(AssociationState.Closed, fsmEvent => { + switch (fsmEvent.FsmEvent) + { + case Status.Failure f: + switch (fsmEvent.StateData) + { + case OutboundUnassociated ou: + ou.StatusCompletionSource.SetException(f.Cause); + return Stop(); + } + + break; + case HandleMsg h: + switch (fsmEvent.StateData) + { + case OutboundUnassociated ou: + /* + * Association has been established, but handshake is not yet complete. + * This actor, the outbound ProtocolStateActor, can now set itself as + * the read handler for the remainder of the handshake process. + */ + var wrappedHandle = h.Handle; + var statusPromise = ou.StatusCompletionSource; + wrappedHandle.ReadHandlerSource.TrySetResult(new ActorHandleEventListener(Self)); + if (SendAssociate(wrappedHandle, _localHandshakeInfo)) + { + _failureDetector.HeartBeat(); + InitHeartbeatTimer(); + // wait for reply from the inbound side of the connection (WaitHandshake) + return + GoTo(AssociationState.WaitHandshake) + .Using(new OutboundUnderlyingAssociated(statusPromise, wrappedHandle)); + } + else + { + //Otherwise, retry + SetTimer("associate-retry", new HandleMsg(wrappedHandle), + RARP.For(Context.System).Provider + .RemoteSettings.BackoffPeriod, repeat: false); + return Stay(); + } + } + + break; + + case DisassociateUnderlying du: + return Stop(); + + + } + State nextState = null; //Transport layer events for outbound associations fsmEvent.FsmEvent.Match() @@ -854,7 +908,7 @@ private void InitializeFSM() if (SendAssociate(wrappedHandle, _localHandshakeInfo)) { _failureDetector.HeartBeat(); - InitTimers(); + InitHeartbeatTimer(); // wait for reply from the inbound side of the connection (WaitHandshake) nextState = GoTo(AssociationState.WaitHandshake) @@ -958,7 +1012,7 @@ private void InitializeFSM() { SendAssociate(wrappedHandle, _localHandshakeInfo); _failureDetector.HeartBeat(); - InitTimers(); + InitHeartbeatTimer(); nextState = GoTo(AssociationState.Open) .Using( @@ -1163,9 +1217,9 @@ protected override void LogTermination(Reason reason) base.LogTermination(reason); } -#endregion + #endregion -#region Actor methods + #region Actor methods /// /// TBD @@ -1176,9 +1230,9 @@ protected override void PostStop() base.PostStop(); //pass to OnTermination } -#endregion + #endregion -#region Internal protocol messaging methods + #region Internal protocol messaging methods private Exception DisassociateException(DisassociateInfo info) { @@ -1250,11 +1304,16 @@ private IAkkaPdu DecodePdu(ByteString pdu) } } - private void InitTimers() + private void InitHeartbeatTimer() { SetTimer("heartbeat-timer", new HeartbeatTimer(), _settings.TransportHeartBeatInterval, true); } + //private void InitHandshakeTimer() + //{ + // SetTimer(handshakeTimerKey, new HandshakeTimer(), _settings.); + //} + private bool SendAssociate(AssociationHandle wrappedHandle, HandshakeInfo info) { try @@ -1299,9 +1358,9 @@ private void PublishError(UnderlyingTransportError transportError) _log.Error(transportError.Cause, transportError.Message); } -#endregion + #endregion -#region Static methods + #region Static methods @@ -1343,7 +1402,7 @@ public static Props InboundProps(HandshakeInfo handshakeInfo, AssociationHandle return Props.Create(() => new ProtocolStateActor(handshakeInfo, wrappedHandle, associationEventListener, settings, codec, failureDetector)); } -#endregion + #endregion } } From d094fcfa7d63943b941561750afd612ac445d5ca Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 18:38:23 -0500 Subject: [PATCH 23/36] adding connect handshake timeout --- .../Akka.Remote.Tests/RemoteConfigSpec.cs | 17 +++++++----- src/core/Akka.Remote/AkkaProtocolSettings.cs | 26 ++++++++++++++----- .../Akka.Remote/Configuration/Remote.conf | 5 ++++ 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemoteConfigSpec.cs b/src/core/Akka.Remote.Tests/RemoteConfigSpec.cs index a6a088865ce..bf832e41e45 100644 --- a/src/core/Akka.Remote.Tests/RemoteConfigSpec.cs +++ b/src/core/Akka.Remote.Tests/RemoteConfigSpec.cs @@ -10,6 +10,7 @@ using System.Linq; using Akka.Actor; using Akka.Actor.Internal; +using Akka.Remote; using Akka.Remote.Transport.DotNetty; using Akka.TestKit; using Akka.Util.Internal; @@ -17,6 +18,7 @@ using System.Net; using Akka.Remote.Transport; using static Akka.Util.RuntimeDetector; +using FluentAssertions; namespace Akka.Remote.Tests { @@ -32,7 +34,7 @@ public RemoteConfigSpec():base(@" [Fact] public void Remoting_should_contain_correct_configuration_values_in_ReferenceConf() { - var remoteSettings = ((RemoteActorRefProvider)((ExtendedActorSystem) Sys).Provider).RemoteSettings; + var remoteSettings = RARP.For(Sys).Provider.RemoteSettings; Assert.False(remoteSettings.LogReceive); Assert.False(remoteSettings.LogSend); @@ -80,7 +82,7 @@ public void Remoting_should_contain_correct_configuration_values_in_ReferenceCon [Fact] public void Remoting_should_be_able_to_parse_AkkaProtocol_related_config_elements() { - var settings = new AkkaProtocolSettings(((RemoteActorRefProvider)((ExtendedActorSystem)Sys).Provider).RemoteSettings.Config); + var settings = new AkkaProtocolSettings(RARP.For(Sys).Provider.RemoteSettings.Config); Assert.Equal(typeof(DeadlineFailureDetector), Type.GetType(settings.TransportFailureDetectorImplementationClass)); Assert.Equal(TimeSpan.FromSeconds(4), settings.TransportHeartBeatInterval); @@ -90,10 +92,11 @@ public void Remoting_should_be_able_to_parse_AkkaProtocol_related_config_element [Fact] public void Remoting_should_contain_correct_heliosTCP_values_in_ReferenceConf() { - var c = ((RemoteActorRefProvider)((ActorSystemImpl)Sys).Provider).RemoteSettings.Config.GetConfig("akka.remote.dot-netty.tcp"); + var c = RARP.For(Sys).Provider.RemoteSettings.Config.GetConfig("akka.remote.dot-netty.tcp"); var s = DotNettyTransportSettings.Create(c); Assert.Equal(TimeSpan.FromSeconds(15), s.ConnectTimeout); + s.ConnectTimeout.Should().Be(new AkkaProtocolSettings(RARP.For(Sys).Provider.RemoteSettings.Config).HandshakeTimeout); Assert.Null(s.WriteBufferHighWaterMark); Assert.Null(s.WriteBufferLowWaterMark); Assert.Equal(256000, s.SendBufferSize.Value); @@ -117,7 +120,7 @@ public void Remoting_should_contain_correct_heliosTCP_values_in_ReferenceConf() public void When_remoting_works_in_Mono_ip_enforcement_should_be_defaulted_to_true() { if (!IsMono) return; // skip IF NOT using Mono - var c = ((RemoteActorRefProvider)((ActorSystemImpl)Sys).Provider).RemoteSettings.Config.GetConfig("akka.remote.dot-netty.tcp"); + var c = RARP.For(Sys).Provider.RemoteSettings.Config.GetConfig("akka.remote.dot-netty.tcp"); var s = DotNettyTransportSettings.Create(c); Assert.True(s.EnforceIpFamily); @@ -127,7 +130,7 @@ public void When_remoting_works_in_Mono_ip_enforcement_should_be_defaulted_to_tr public void When_remoting_works_not_in_Mono_ip_enforcement_should_be_defaulted_to_false() { if (IsMono) return; // skip IF using Mono - var c = ((RemoteActorRefProvider)((ActorSystemImpl)Sys).Provider).RemoteSettings.Config.GetConfig("akka.remote.dot-netty.tcp"); + var c = RARP.For(Sys).Provider.RemoteSettings.Config.GetConfig("akka.remote.dot-netty.tcp"); var s = DotNettyTransportSettings.Create(c); Assert.False(s.EnforceIpFamily); @@ -137,7 +140,7 @@ public void When_remoting_works_not_in_Mono_ip_enforcement_should_be_defaulted_t [Fact] public void Remoting_should_contain_correct_socket_worker_pool_configuration_values_in_ReferenceConf() { - var c = ((RemoteActorRefProvider)((ActorSystemImpl)Sys).Provider).RemoteSettings.Config.GetConfig("akka.remote.dot-netty.tcp"); + var c = RARP.For(Sys).Provider.RemoteSettings.Config.GetConfig("akka.remote.dot-netty.tcp"); // server-socket-worker-pool { @@ -159,7 +162,7 @@ public void Remoting_should_contain_correct_socket_worker_pool_configuration_val [Fact] public void Remoting_should_contain_correct_hostname_values_in_ReferenceConf() { - var c = ((RemoteActorRefProvider)((ActorSystemImpl)Sys).Provider).RemoteSettings.Config.GetConfig("akka.remote.dot-netty.tcp"); + var c = RARP.For(Sys).Provider.RemoteSettings.Config.GetConfig("akka.remote.dot-netty.tcp"); var s = DotNettyTransportSettings.Create(c); //Non-specified hostnames should default to IPAddress.Any diff --git a/src/core/Akka.Remote/AkkaProtocolSettings.cs b/src/core/Akka.Remote/AkkaProtocolSettings.cs index 0e478c94822..edb4bab6508 100644 --- a/src/core/Akka.Remote/AkkaProtocolSettings.cs +++ b/src/core/Akka.Remote/AkkaProtocolSettings.cs @@ -11,34 +11,48 @@ namespace Akka.Remote { /// - /// TBD + /// Setings for the AkkaProtocolTransport /// public class AkkaProtocolSettings { /// - /// TBD + /// The HOCON for the failure detector. /// public Config TransportFailureDetectorConfig { get; private set; } /// - /// TBD + /// The failure detection implementation class FQN. /// public string TransportFailureDetectorImplementationClass { get; private set; } /// - /// TBD + /// The heartbeat interval used to keep the protocol transport alive. /// public TimeSpan TransportHeartBeatInterval { get; private set; } /// - /// TBD + /// The heartbeat handshake timeout. /// - /// TBD + public TimeSpan HandshakeTimeout { get; private set; } + + /// + /// Creates a new AkkaProtocolSettings instance. + /// + /// The HOCON configuration. public AkkaProtocolSettings(Config config) { TransportFailureDetectorConfig = config.GetConfig("akka.remote.transport-failure-detector"); TransportFailureDetectorImplementationClass = TransportFailureDetectorConfig.GetString("implementation-class"); TransportHeartBeatInterval = TransportFailureDetectorConfig.GetTimeSpan("heartbeat-interval"); + + // backwards compatibility with the existing dot-netty.tcp.connection-timeout + var enabledTransports = config.GetStringList("akka.remote.enabled-transports"); + if (enabledTransports.Contains("akka.remote.dot-netty.tcp")) + HandshakeTimeout = config.GetTimeSpan("akka.remote.dot-netty.tcp.connection-timeout"); + else if (enabledTransports.Contains("akka.remote.dot-netty.ssl")) + HandshakeTimeout = config.GetTimeSpan("akka.remote.dot-netty.ssl.connection-timeout"); + else + HandshakeTimeout = config.GetTimeSpan("akka.remote.handshake-timeout", allowInfinite:false); } } } diff --git a/src/core/Akka.Remote/Configuration/Remote.conf b/src/core/Akka.Remote/Configuration/Remote.conf index ff207b4e207..debe14c4fc6 100644 --- a/src/core/Akka.Remote/Configuration/Remote.conf +++ b/src/core/Akka.Remote/Configuration/Remote.conf @@ -121,6 +121,11 @@ akka { # Acknowledgment timeout of management commands sent to the transport stack. command-ack-timeout = 30 s + # The timeout for outbound associations to perform the handshake. + # If the transport is akka.remote.dot-netty.tcp or akka.remote.dot-netty.ssl + # the configured connection-timeout for the transport will be used instead. + handshake-timeout = 15 s + # If set to a nonempty string remoting will use the given dispatcher for # its internal actors otherwise the default dispatcher is used. Please note # that since remoting can load arbitrary 3rd party drivers (see From ca608313db965fcb4f2137515be7515775eca355 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 20:33:16 -0500 Subject: [PATCH 24/36] working on updating ProtocolStateActors --- src/Akka.sln.DotSettings | 1 + .../Akka.Remote.Tests/RemoteConfigSpec.cs | 3 - .../Transport/AkkaProtocolTransport.cs | 244 +++++++----------- 3 files changed, 96 insertions(+), 152 deletions(-) diff --git a/src/Akka.sln.DotSettings b/src/Akka.sln.DotSettings index a83f932f2dc..41a06c2ddfc 100644 --- a/src/Akka.sln.DotSettings +++ b/src/Akka.sln.DotSettings @@ -19,6 +19,7 @@ True True True + True True True True diff --git a/src/core/Akka.Remote.Tests/RemoteConfigSpec.cs b/src/core/Akka.Remote.Tests/RemoteConfigSpec.cs index bf832e41e45..3114b54c868 100644 --- a/src/core/Akka.Remote.Tests/RemoteConfigSpec.cs +++ b/src/core/Akka.Remote.Tests/RemoteConfigSpec.cs @@ -8,9 +8,6 @@ using System; using System.Collections.Generic; using System.Linq; -using Akka.Actor; -using Akka.Actor.Internal; -using Akka.Remote; using Akka.Remote.Transport.DotNetty; using Akka.TestKit; using Akka.Util.Internal; diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index e59d55721ae..34dadf7b2ed 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -845,7 +845,7 @@ private void InitializeFSM() return Stop(); } - break; + return Stay(); case HandleMsg h: switch (fsmEvent.StateData) { @@ -877,161 +877,112 @@ private void InitializeFSM() } } - break; - + return Stay(); case DisassociateUnderlying du: return Stop(); - - + case HandshakeTimer t when fsmEvent.StateData is OutboundUnassociated ou: + var errMsg = $"No response from remote for outbound association. Associate timed out after " + + $"[{_settings.HandshakeTimeout.TotalMilliseconds} ms]."; + return Stop(new Failure(new TimeoutReason(errMsg))); + default: + return Stay(); } - - State nextState = null; - //Transport layer events for outbound associations - fsmEvent.FsmEvent.Match() - .With(f => fsmEvent.StateData.Match() - .With(ou => - { - ou.StatusCompletionSource.SetException(f.Cause); - nextState = Stop(); - })) - .With(h => fsmEvent.StateData.Match() - .With(ou => - { - /* - * Association has been established, but handshake is not yet complete. - * This actor, the outbound ProtocolStateActor, can now set itself as - * the read handler for the remainder of the handshake process. - */ - AssociationHandle wrappedHandle = h.Handle; - var statusPromise = ou.StatusCompletionSource; - wrappedHandle.ReadHandlerSource.TrySetResult(new ActorHandleEventListener(Self)); - if (SendAssociate(wrappedHandle, _localHandshakeInfo)) - { - _failureDetector.HeartBeat(); - InitHeartbeatTimer(); - // wait for reply from the inbound side of the connection (WaitHandshake) - nextState = - GoTo(AssociationState.WaitHandshake) - .Using(new OutboundUnderlyingAssociated(statusPromise, wrappedHandle)); - } - else - { - //Otherwise, retry - SetTimer("associate-retry", new HandleMsg(wrappedHandle), - RARP.For(Context.System).Provider - .RemoteSettings.BackoffPeriod, repeat: false); - nextState = Stay(); - } - })) - .With(d => - { - nextState = Stop(); - }) - .Default(m => { nextState = Stay(); }); - - return nextState; }); //Transport layer events for outbound associations When(AssociationState.WaitHandshake, @event => { - State nextState = null; - - @event.FsmEvent.Match() - .With(e => - { - PublishError(e); - nextState = Stay(); - }) - .With(d => - { - nextState = Stop(new Failure(d.Info)); - }) - .With(m => - { - var pdu = DecodePdu(m.Payload); - @event.StateData.Match() - .With(ola => - { - /* - * This state is used for OutboundProtocolState actors when they receive - * a reply back from the inbound end of the association. - */ - var wrappedHandle = ola.WrappedHandle; - var statusCompletionSource = ola.StatusCompletionSource; - pdu.Match() - .With(a => - { - - var handshakeInfo = a.Info; - if (_refuseUid.HasValue && _refuseUid == handshakeInfo.Uid) //refused UID - { - SendDisassociate(wrappedHandle, DisassociateInfo.Quarantined); - nextState = Stop(new Failure(new ForbiddenUidReason())); - } - else //accepted UID - { - _failureDetector.HeartBeat(); - nextState = - GoTo(AssociationState.Open) - .Using( - new AssociatedWaitHandler( - NotifyOutboundHandler(wrappedHandle, handshakeInfo, - statusCompletionSource), wrappedHandle, - new Queue())); - } - }) - .With(d => - { - //After receiving Disassociate we MUST NOT send back a Disassociate (loop) - nextState = Stop(new Failure(d.Reason)); - }) - .Default(d => - { - _log.Debug("Expected message of type Associate; instead received {0}", d); - //Expect handshake to be finished, dropping connection - SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); - nextState = Stop(); - }); - }) - .With(iu => + switch (@event.FsmEvent) + { + case Disassociated d: + return Stop(new Failure(d.Info)); + case InboundPayload p when @event.StateData is OutboundUnderlyingAssociated ola: + { + var pdu = DecodePdu(p.Payload); + /* + * This state is used for OutboundProtocolState actors when they receive + * a reply back from the inbound end of the association. + */ + var wrappedHandle = ola.WrappedHandle; + var statusCompletionSource = ola.StatusCompletionSource; + switch (pdu) { - /* - * This state is used by inbound protocol state actors - * when they receive an association attempt from the - * outbound side of the association. - */ - var associationHandler = iu.AssociationEventListener; - var wrappedHandle = iu.WrappedHandle; - pdu.Match() - .With(d => + case Associate a: + var handshakeInfo = a.Info; + if (_refuseUid.HasValue && _refuseUid == handshakeInfo.Uid) //refused UID { - nextState = Stop(new Failure(d.Reason)); - }) - .With(a => + SendDisassociate(wrappedHandle, DisassociateInfo.Quarantined); + return Stop(new Failure(new ForbiddenUidReason())); + } + else //accepted UID { - SendAssociate(wrappedHandle, _localHandshakeInfo); _failureDetector.HeartBeat(); - InitHeartbeatTimer(); - nextState = + CancelTimer(handshakeTimerKey); + return GoTo(AssociationState.Open) .Using( new AssociatedWaitHandler( - NotifyInboundHandler(wrappedHandle, a.Info, associationHandler), - wrappedHandle, new Queue())); - }) - .Default(d => - { - SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); - nextState = Stop(); - }); - }); + NotifyOutboundHandler(wrappedHandle, handshakeInfo, + statusCompletionSource), wrappedHandle, + new Queue())); + } + case Disassociate d: + //After receiving Disassociate we MUST NOT send back a Disassociate (loop) + return Stop(new Failure(d.Reason)); + default: + _log.Debug("Expected message of type Associate; instead received {0}", @event.FsmEvent); + //Expect handshake to be finished, dropping connection + SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); + return Stop(); + } + } - }) - .With(h => @event.StateData.Match() - .With(ou => nextState = HandleTimers(ou.WrappedHandle))); + case HeartbeatTimer t when @event.FsmEvent is OutboundUnderlyingAssociated oua: + return HandleTimers(oua.WrappedHandle); - return nextState; + // Events for inbound associations + case InboundPayload p when @event.StateData is InboundUnassociated iu: + { + var pdu = DecodePdu(p.Payload); + /* + * This state is used by inbound protocol state actors + * when they receive an association attempt from the + * outbound side of the association. + */ + var associationHandler = iu.AssociationEventListener; + var wrappedHandle = iu.WrappedHandle; + switch (pdu) + { + case Associate a when _refuseUid.HasValue && _refuseUid == a.Info.Uid: + SendDisassociate(wrappedHandle, DisassociateInfo.Quarantined); + return Stop(new Failure(new ForbiddenUidReason())); + case Associate a: + _failureDetector.HeartBeat(); + CancelTimer(handshakeTimerKey); + return GoTo(AssociationState.Open).Using( + new AssociatedWaitHandler( + NotifyInboundHandler(wrappedHandle, a.Info, associationHandler), + wrappedHandle, new Queue())); + case Disassociate d: + // After receiving Disassociate we MUST NOT send back a Disassociate (loop) + return Stop(new Failure(d.Reason)); + default: + if (_log.IsDebugEnabled) + _log.Debug("Sending disassociate to [{0}] because unexpected message of type [{1}] was received during handshake.", wrappedHandle, @event.FsmEvent.GetType()); + SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); + return Stop(); + } + } + + case HandshakeTimer t when @event.StateData is OutboundUnderlyingAssociated oua: + if (_log.IsDebugEnabled) + _log.Debug("Sending disassociate to [{0}] because handshake timed out for outbound association after [{1}] ms.", oua.WrappedHandle, _settings.HandshakeTimeout.TotalMilliseconds); + SendDisassociate(oua.WrappedHandle, DisassociateInfo.Unknown); + return Stop(new Failure(new TimeoutReason( + $"No response from remote for outbound association. Handshake timed out after [{_settings.HandshakeTimeout.TotalMilliseconds}] ms"))); + default: + return null; + } }); When(AssociationState.Open, @event => @@ -1194,6 +1145,7 @@ private void InitializeFSM() // therefore we just have to set ourselves as listener and wait for // incoming handshake attempts from the client. d.WrappedHandle.ReadHandlerSource.SetResult(new ActorHandleEventListener(Self)); + InitHandshakeTimer(); StartWith(AssociationState.WaitHandshake, d); }); @@ -1219,18 +1171,12 @@ protected override void LogTermination(Reason reason) #endregion - #region Actor methods - - /// - /// TBD - /// protected override void PostStop() { CancelTimer("heartbeat-timer"); base.PostStop(); //pass to OnTermination } - #endregion #region Internal protocol messaging methods @@ -1309,10 +1255,10 @@ private void InitHeartbeatTimer() SetTimer("heartbeat-timer", new HeartbeatTimer(), _settings.TransportHeartBeatInterval, true); } - //private void InitHandshakeTimer() - //{ - // SetTimer(handshakeTimerKey, new HandshakeTimer(), _settings.); - //} + private void InitHandshakeTimer() + { + SetTimer(handshakeTimerKey, new HandshakeTimer(), _settings.HandshakeTimeout, true); + } private bool SendAssociate(AssociationHandle wrappedHandle, HandshakeInfo info) { From 81e960d9d0a74314446b1e60f0fc48b2938085e3 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 25 Apr 2019 20:46:06 -0500 Subject: [PATCH 25/36] WIP --- .../Transport/AkkaProtocolTransport.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index 34dadf7b2ed..877a2487bf9 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -980,6 +980,9 @@ private void InitializeFSM() SendDisassociate(oua.WrappedHandle, DisassociateInfo.Unknown); return Stop(new Failure(new TimeoutReason( $"No response from remote for outbound association. Handshake timed out after [{_settings.HandshakeTimeout.TotalMilliseconds}] ms"))); + case UnderlyingTransportError ue: + PublishError(ue); + return Stay(); default: return null; } @@ -987,6 +990,40 @@ private void InitializeFSM() When(AssociationState.Open, @event => { + switch (@event.FsmEvent) + { + case Disassociated d: + return Stop(new Failure(d.Info)); + case InboundPayload ip: + { + var pdu = DecodePdu(ip.Payload); + switch (pdu) + { + case Disassociate d: + return Stop(new Failure(d.Reason)); + case Heartbeat h: + _failureDetector.HeartBeat(); + return Stay(); + case Payload p: + _failureDetector.HeartBeat(); + switch (@event.StateData) + { + case AssociatedWaitHandler awh: + var nQueue = new Queue(awh.Queue); + nQueue.Enqueue(p.Bytes); + return + Stay() + .Using(new AssociatedWaitHandler(awh.HandlerListener, awh.WrappedHandle, + nQueue)); + case ListenerReady lr: + lr.Listener.Notify(new InboundPayload(p.Bytes)); + return Stay(); + default: + throw new AkkaProtocolException("Unhandled message in state") + } + } + } + } State nextState = null; @event.FsmEvent.Match() .With(e => From f52847c6a14473ae7e77147f682144215e4f7c6d Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 26 Apr 2019 09:26:50 -0500 Subject: [PATCH 26/36] finished most AkkaProtocolStateActor ports --- .../Transport/AkkaProtocolTransport.cs | 173 +++++++----------- 1 file changed, 69 insertions(+), 104 deletions(-) diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index 877a2487bf9..acf55b66618 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -990,117 +990,82 @@ private void InitializeFSM() When(AssociationState.Open, @event => { - switch (@event.FsmEvent) - { - case Disassociated d: - return Stop(new Failure(d.Info)); - case InboundPayload ip: - { - var pdu = DecodePdu(ip.Payload); - switch (pdu) + switch (@event.FsmEvent) + { + case Disassociated d: + return Stop(new Failure(d.Info)); + case InboundPayload ip: { - case Disassociate d: - return Stop(new Failure(d.Reason)); - case Heartbeat h: - _failureDetector.HeartBeat(); - return Stay(); - case Payload p: - _failureDetector.HeartBeat(); - switch (@event.StateData) - { - case AssociatedWaitHandler awh: - var nQueue = new Queue(awh.Queue); - nQueue.Enqueue(p.Bytes); - return - Stay() - .Using(new AssociatedWaitHandler(awh.HandlerListener, awh.WrappedHandle, - nQueue)); - case ListenerReady lr: - lr.Listener.Notify(new InboundPayload(p.Bytes)); - return Stay(); - default: - throw new AkkaProtocolException("Unhandled message in state") - } - } - } - } - State nextState = null; - @event.FsmEvent.Match() - .With(e => - { - PublishError(e); - nextState = Stay(); - }) - .With(d => - { - nextState = Stop(new Failure(d.Info)); - }) - .With(ip => - { - var pdu = DecodePdu(ip.Payload); - pdu.Match() - .With(d => - { - nextState = Stop(new Failure(d.Reason)); - }) - .With(h => - { - _failureDetector.HeartBeat(); - nextState = Stay(); - }) - .With(p => + var pdu = DecodePdu(ip.Payload); + switch (pdu) { - _failureDetector.HeartBeat(); - @event.StateData.Match() - .With(awh => - { - var nQueue = new Queue(awh.Queue); - nQueue.Enqueue(p.Bytes); - nextState = - Stay() - .Using(new AssociatedWaitHandler(awh.HandlerListener, awh.WrappedHandle, - nQueue)); - }) - .With(lr => - { - lr.Listener.Notify(new InboundPayload(p.Bytes)); - nextState = Stay(); - }) - .Default(msg => + case Disassociate d: + return Stop(new Failure(d.Reason)); + case Heartbeat h: + _failureDetector.HeartBeat(); + return Stay(); + case Payload p: + _failureDetector.HeartBeat(); + switch (@event.StateData) { - throw new AkkaProtocolException($"Unhandled message in state Open(InboundPayload) with type {msg.GetType()}"); - }); - }) - .Default(d => - { - nextState = Stay(); - }); - }) - .With(hrt => @event.StateData.Match() - .With(awh => nextState = HandleTimers(awh.WrappedHandle)) - .With(lr => nextState = HandleTimers(lr.WrappedHandle))) - .With(du => + case AssociatedWaitHandler awh: + var nQueue = new Queue(awh.Queue); + nQueue.Enqueue(p.Bytes); + return + Stay() + .Using(new AssociatedWaitHandler(awh.HandlerListener, awh.WrappedHandle, + nQueue)); + case ListenerReady lr: + lr.Listener.Notify(new InboundPayload(p.Bytes)); + return Stay(); + default: + throw new AkkaProtocolException( + $"Unhandled message in state Open(InboundPayload) with type [{@event.FsmEvent.GetType()}]"); + } + default: + return Stay(); + } + } + case HeartbeatTimer ht when @event.StateData is AssociatedWaitHandler awh: + return HandleTimers(awh.WrappedHandle); + case HeartbeatTimer ht when @event.StateData is ListenerReady lr: + return HandleTimers(lr.WrappedHandle); + case DisassociateUnderlying dl: { - AssociationHandle handle = null; - @event.StateData.Match() - .With(lr => handle = lr.WrappedHandle) - .With(awh => handle = awh.WrappedHandle) - .Default(msg => + AssociationHandle GetHandle(ProtocolStateData data) + { + switch (data) { - throw new AkkaProtocolException($"unhandled message in state Open(DisassociateUnderlying) with type {msg.GetType()}"); - }); - SendDisassociate(handle, du.Info); - nextState = Stop(); - }) - .With(hlr => @event.StateData.Match() - .With(awh => + case ListenerReady lr: + return lr.WrappedHandle; + case AssociatedWaitHandler awh: + return awh.WrappedHandle; + default: + throw new AkkaProtocolException( + $"Unhandled message in state Open(DisassociateUnderlying) with type [{@event.FsmEvent.GetType()}]"); + } + } + + var handle = GetHandle(@event.StateData); + + // No debug logging here as sending DisassociateUnderlying(Unknown) should have been logged from where + // it was sent + SendDisassociate(handle, dl.Info); + return Stop(); + } + case HandleListenerRegistered hlr when @event.StateData is AssociatedWaitHandler awh: + foreach (var p in awh.Queue) { - foreach (var msg in awh.Queue) - hlr.Listener.Notify(new InboundPayload(msg)); - nextState = Stay().Using(new ListenerReady(hlr.Listener, awh.WrappedHandle)); - })); + hlr.Listener.Notify(new InboundPayload(p)); + } - return nextState; + return Stay().Using(new ListenerReady(hlr.Listener, awh.WrappedHandle)); + case UnderlyingTransportError e: + PublishError(e); + return Stay(); + default: + return null; + } }); OnTermination(@event => @event.StateData.Match() From 8835426d23a4efb1b8525d6c18fc82d44b728f6b Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 26 Apr 2019 10:50:51 -0500 Subject: [PATCH 27/36] fixed bug when handling inbound associations --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 19 +++- .../Transport/AkkaProtocolSpec.cs | 13 ++- src/core/Akka.Remote/AkkaProtocolSettings.cs | 2 +- .../Transport/AkkaProtocolTransport.cs | 88 ++++++++++--------- 4 files changed, 72 insertions(+), 50 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index dccf4e2941f..7eaa885e086 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -38,6 +38,7 @@ public RemotingSpec(ITestOutputHelper helper) : base(GetConfig(), helper) var conf = c2.WithFallback(c1); //ConfigurationFactory.ParseString(GetOtherRemoteSysConfig()); _remoteSystem = ActorSystem.Create("remote-sys", conf); + InitializeLogger(_remoteSystem); Deploy(Sys, new Deploy(@"/gonk", new RemoteScope(Addr(_remoteSystem, "tcp")))); Deploy(Sys, new Deploy(@"/zagzag", new RemoteScope(Addr(_remoteSystem, "udp")))); @@ -56,7 +57,14 @@ private static string GetConfig() } akka { - actor.provider = ""Akka.Remote.RemoteActorRefProvider,Akka.Remote"" + actor.provider = remote + loglevel = DEBUG + + debug{ + unhandled = on + fsm = on + receive = on + } remote { transport = ""Akka.Remote.Remoting,Akka.Remote"" @@ -102,8 +110,13 @@ protected string GetOtherRemoteSysConfig() } akka { - actor.provider = ""Akka.Remote.RemoteActorRefProvider,Akka.Remote"" - + actor.provider = remote + loglevel = DEBUG + debug{ + unhandled = on + fsm = on + receive = on + } remote { transport = ""Akka.Remote.Remoting,Akka.Remote"" diff --git a/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs b/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs index c01030d0a22..8ee1e5816ab 100644 --- a/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs +++ b/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs @@ -16,6 +16,7 @@ using Akka.Util.Internal; using Google.Protobuf; using Xunit; +using Xunit.Abstractions; using SerializedMessage = Akka.Remote.Serialization.Proto.Msg.Payload; namespace Akka.Remote.Tests.Transport @@ -45,8 +46,8 @@ public class AkkaProtocolSpec : AkkaSpec private IHandleEvent testAssociate(int uid) { return new InboundPayload(codec.ConstructAssociate(new HandshakeInfo(remoteAkkaAddress, uid))); } private TimeSpan DefaultTimeout { get { return Dilated(TestKitSettings.DefaultTimeout); } } - public AkkaProtocolSpec() - : base(@"akka.test.default-timeout = 1.5 s") + public AkkaProtocolSpec(ITestOutputHelper helper) + : base(@"akka.test.default-timeout = 1.5 s", helper) { codec = new AkkaPduProtobuffCodec(Sys); testEnvelope = codec.ConstructMessage(localAkkaAddress, TestActor, testMsg); @@ -105,7 +106,11 @@ public override void HeartBeat() } private Config config = ConfigurationFactory.ParseString( - @"akka.remote { + @"akka{ + loglevel = DEBUG + actor.debug.unhandled = on + actor.debug.fsm = on + remote { transport-failure-detector { implementation-class = ""Akka.Remote.PhiAccrualFailureDetector, Akka.Remote"" @@ -127,7 +132,7 @@ public override void HeartBeat() startup-timeout = 5 s use-passive-connections = on - }"); + }}"); #endregion diff --git a/src/core/Akka.Remote/AkkaProtocolSettings.cs b/src/core/Akka.Remote/AkkaProtocolSettings.cs index edb4bab6508..58cd2ce24b3 100644 --- a/src/core/Akka.Remote/AkkaProtocolSettings.cs +++ b/src/core/Akka.Remote/AkkaProtocolSettings.cs @@ -52,7 +52,7 @@ public AkkaProtocolSettings(Config config) else if (enabledTransports.Contains("akka.remote.dot-netty.ssl")) HandshakeTimeout = config.GetTimeSpan("akka.remote.dot-netty.ssl.connection-timeout"); else - HandshakeTimeout = config.GetTimeSpan("akka.remote.handshake-timeout", allowInfinite:false); + HandshakeTimeout = config.GetTimeSpan("akka.remote.handshake-timeout", TimeSpan.FromSeconds(20), allowInfinite:false); } } } diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index acf55b66618..12793db43c3 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -729,15 +729,15 @@ internal class ForbiddenUidReason { } internal class ProtocolStateActor : FSM { private readonly ILoggingAdapter _log = Context.GetLogger(); - public InitialProtocolStateData _initialData; + private readonly InitialProtocolStateData _initialData; private readonly HandshakeInfo _localHandshakeInfo; private int? _refuseUid; - private AkkaProtocolSettings _settings; - private Address _localAddress; - private AkkaPduCodec _codec; - private FailureDetector _failureDetector; + private readonly AkkaProtocolSettings _settings; + private readonly Address _localAddress; + private readonly AkkaPduCodec _codec; + private readonly FailureDetector _failureDetector; - private const string handshakeTimerKey = "handshake-timer"; + private const string HandshakeTimerKey = "handshake-timer"; /// /// Constructor for outbound ProtocolStateActors @@ -917,7 +917,7 @@ private void InitializeFSM() else //accepted UID { _failureDetector.HeartBeat(); - CancelTimer(handshakeTimerKey); + CancelTimer(HandshakeTimerKey); return GoTo(AssociationState.Open) .Using( @@ -930,14 +930,16 @@ private void InitializeFSM() //After receiving Disassociate we MUST NOT send back a Disassociate (loop) return Stop(new Failure(d.Reason)); default: - _log.Debug("Expected message of type Associate; instead received {0}", @event.FsmEvent); //Expect handshake to be finished, dropping connection + if (_log.IsDebugEnabled) + _log.Debug("Expected message of type Associate; instead received {0}", @event.FsmEvent.GetType()); + SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); return Stop(); } } - case HeartbeatTimer t when @event.FsmEvent is OutboundUnderlyingAssociated oua: + case HeartbeatTimer t when @event.StateData is OutboundUnderlyingAssociated oua: return HandleTimers(oua.WrappedHandle); // Events for inbound associations @@ -953,22 +955,19 @@ private void InitializeFSM() var wrappedHandle = iu.WrappedHandle; switch (pdu) { - case Associate a when _refuseUid.HasValue && _refuseUid == a.Info.Uid: - SendDisassociate(wrappedHandle, DisassociateInfo.Quarantined); - return Stop(new Failure(new ForbiddenUidReason())); case Associate a: + SendAssociate(wrappedHandle, _localHandshakeInfo); _failureDetector.HeartBeat(); - CancelTimer(handshakeTimerKey); + CancelTimer(HandshakeTimerKey); return GoTo(AssociationState.Open).Using( new AssociatedWaitHandler( NotifyInboundHandler(wrappedHandle, a.Info, associationHandler), wrappedHandle, new Queue())); - case Disassociate d: - // After receiving Disassociate we MUST NOT send back a Disassociate (loop) - return Stop(new Failure(d.Reason)); + + // Got a stray message -- explicitly reset the association (force remote endpoint to reassociate) default: if (_log.IsDebugEnabled) - _log.Debug("Sending disassociate to [{0}] because unexpected message of type [{1}] was received during handshake.", wrappedHandle, @event.FsmEvent.GetType()); + _log.Debug("Sending disassociate to [{0}] because unexpected message of type [{1}] was received unassociated.", wrappedHandle, @event.FsmEvent.GetType()); SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); return Stop(); } @@ -1031,28 +1030,28 @@ private void InitializeFSM() case HeartbeatTimer ht when @event.StateData is ListenerReady lr: return HandleTimers(lr.WrappedHandle); case DisassociateUnderlying dl: - { - AssociationHandle GetHandle(ProtocolStateData data) { - switch (data) + AssociationHandle GetHandle(ProtocolStateData data) { - case ListenerReady lr: - return lr.WrappedHandle; - case AssociatedWaitHandler awh: - return awh.WrappedHandle; - default: - throw new AkkaProtocolException( - $"Unhandled message in state Open(DisassociateUnderlying) with type [{@event.FsmEvent.GetType()}]"); + switch (data) + { + case ListenerReady lr: + return lr.WrappedHandle; + case AssociatedWaitHandler awh: + return awh.WrappedHandle; + default: + throw new AkkaProtocolException( + $"Unhandled message in state Open(DisassociateUnderlying) with type [{@event.FsmEvent.GetType()}]"); + } } - } - var handle = GetHandle(@event.StateData); + var handle = GetHandle(@event.StateData); - // No debug logging here as sending DisassociateUnderlying(Unknown) should have been logged from where - // it was sent - SendDisassociate(handle, dl.Info); - return Stop(); - } + // No debug logging here as sending DisassociateUnderlying(Unknown) should have been logged from where + // it was sent + SendDisassociate(handle, dl.Info); + return Stop(); + } case HandleListenerRegistered hlr when @event.StateData is AssociatedWaitHandler awh: foreach (var p in awh.Queue) { @@ -1147,10 +1146,10 @@ AssociationHandle GetHandle(ProtocolStateData data) // therefore we just have to set ourselves as listener and wait for // incoming handshake attempts from the client. d.WrappedHandle.ReadHandlerSource.SetResult(new ActorHandleEventListener(Self)); - InitHandshakeTimer(); + StartWith(AssociationState.WaitHandshake, d); }); - + InitHandshakeTimer(); } /// @@ -1159,8 +1158,7 @@ AssociationHandle GetHandle(ProtocolStateData data) /// TBD protected override void LogTermination(Reason reason) { - var failure = reason as Failure; - if (failure != null) + if (reason is Failure failure) { failure.Cause.Match() .With(() => { }) //no logging @@ -1182,7 +1180,7 @@ protected override void PostStop() #region Internal protocol messaging methods - private Exception DisassociateException(DisassociateInfo info) + private static Exception DisassociateException(DisassociateInfo info) { switch (info) { @@ -1205,9 +1203,14 @@ private State HandleTimers(AssociationHandl } else { + if (_log.IsDebugEnabled) + { + _log.Debug("Sending disassociate to [{0}] because failure detector triggered in state [{1}]", wrappedHandle, StateName); + } + //send disassociate just to be sure SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); - return Stop(new Failure(new TimeoutReason("No response from remote. Handshake timed out or transport failure detector triggered."))); + return Stop(new Failure(new TimeoutReason($"No response from remote. Handshake timed out or transport failure detector triggered. (internal state was {StateName})"))); } } @@ -1223,7 +1226,8 @@ private Task NotifyOutboundHandler(AssociationHandle wrapp var readHandlerPromise = new TaskCompletionSource(); ListenForListenerRegistration(readHandlerPromise); - statusPromise.SetResult(new AkkaProtocolHandle(_localAddress, wrappedHandle.RemoteAddress, readHandlerPromise, wrappedHandle, handshakeInfo, Self, _codec)); + statusPromise.SetResult(new AkkaProtocolHandle(_localAddress, wrappedHandle.RemoteAddress, + readHandlerPromise, wrappedHandle, handshakeInfo, Self, _codec)); return readHandlerPromise.Task; } @@ -1259,7 +1263,7 @@ private void InitHeartbeatTimer() private void InitHandshakeTimer() { - SetTimer(handshakeTimerKey, new HandshakeTimer(), _settings.HandshakeTimeout, true); + SetTimer(HandshakeTimerKey, new HandshakeTimer(), _settings.HandshakeTimeout, true); } private bool SendAssociate(AssociationHandle wrappedHandle, HandshakeInfo info) From aadec2a2af4affe6fe73340d25c48d2629f3bf43 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 26 Apr 2019 11:29:28 -0500 Subject: [PATCH 28/36] cleaning up AkkaProtocolState actors --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 4 +- .../Transport/AkkaProtocolSpec.cs | 4 +- .../Transport/AkkaProtocolTransport.cs | 69 +++++++++++-------- 3 files changed, 43 insertions(+), 34 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index 7eaa885e086..a4014489b77 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -60,7 +60,7 @@ private static string GetConfig() actor.provider = remote loglevel = DEBUG - debug{ + actor.debug{ unhandled = on fsm = on receive = on @@ -112,7 +112,7 @@ protected string GetOtherRemoteSysConfig() akka { actor.provider = remote loglevel = DEBUG - debug{ + actor.debug{ unhandled = on fsm = on receive = on diff --git a/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs b/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs index 8ee1e5816ab..8894d71f52b 100644 --- a/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs +++ b/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs @@ -7,6 +7,7 @@ using System; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Akka.Actor; using Akka.Configuration; @@ -107,9 +108,6 @@ public override void HeartBeat() private Config config = ConfigurationFactory.ParseString( @"akka{ - loglevel = DEBUG - actor.debug.unhandled = on - actor.debug.fsm = on remote { transport-failure-detector { diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index 12793db43c3..16848c22f2d 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -835,6 +835,7 @@ private void InitializeFSM() { When(AssociationState.Closed, fsmEvent => { + _log.Info("{0} - {1}: {2}", StateName, StateData, @fsmEvent.FsmEvent); switch (fsmEvent.FsmEvent) { case Status.Failure f: @@ -845,7 +846,7 @@ private void InitializeFSM() return Stop(); } - return Stay(); + return null; case HandleMsg h: switch (fsmEvent.StateData) { @@ -869,7 +870,7 @@ private void InitializeFSM() } else { - //Otherwise, retry + // Underlying transport was busy -- Associate could not be sent SetTimer("associate-retry", new HandleMsg(wrappedHandle), RARP.For(Context.System).Provider .RemoteSettings.BackoffPeriod, repeat: false); @@ -883,6 +884,7 @@ private void InitializeFSM() case HandshakeTimer t when fsmEvent.StateData is OutboundUnassociated ou: var errMsg = $"No response from remote for outbound association. Associate timed out after " + $"[{_settings.HandshakeTimeout.TotalMilliseconds} ms]."; + ou.StatusCompletionSource.SetException(new TimeoutException(errMsg)); return Stop(new Failure(new TimeoutReason(errMsg))); default: return Stay(); @@ -892,6 +894,7 @@ private void InitializeFSM() //Transport layer events for outbound associations When(AssociationState.WaitHandshake, @event => { + _log.Info("{0} - {1}: {2}", StateName, StateData, @event.FsmEvent); switch (@event.FsmEvent) { case Disassociated d: @@ -944,34 +947,34 @@ private void InitializeFSM() // Events for inbound associations case InboundPayload p when @event.StateData is InboundUnassociated iu: + { + var pdu = DecodePdu(p.Payload); + /* + * This state is used by inbound protocol state actors + * when they receive an association attempt from the + * outbound side of the association. + */ + var associationHandler = iu.AssociationEventListener; + var wrappedHandle = iu.WrappedHandle; + switch (pdu) { - var pdu = DecodePdu(p.Payload); - /* - * This state is used by inbound protocol state actors - * when they receive an association attempt from the - * outbound side of the association. - */ - var associationHandler = iu.AssociationEventListener; - var wrappedHandle = iu.WrappedHandle; - switch (pdu) - { - case Associate a: - SendAssociate(wrappedHandle, _localHandshakeInfo); - _failureDetector.HeartBeat(); - CancelTimer(HandshakeTimerKey); - return GoTo(AssociationState.Open).Using( - new AssociatedWaitHandler( - NotifyInboundHandler(wrappedHandle, a.Info, associationHandler), - wrappedHandle, new Queue())); - - // Got a stray message -- explicitly reset the association (force remote endpoint to reassociate) - default: - if (_log.IsDebugEnabled) - _log.Debug("Sending disassociate to [{0}] because unexpected message of type [{1}] was received unassociated.", wrappedHandle, @event.FsmEvent.GetType()); - SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); - return Stop(); - } + case Associate a: + SendAssociate(wrappedHandle, _localHandshakeInfo); + _failureDetector.HeartBeat(); + CancelTimer(HandshakeTimerKey); + return GoTo(AssociationState.Open).Using( + new AssociatedWaitHandler( + NotifyInboundHandler(wrappedHandle, a.Info, associationHandler), + wrappedHandle, new Queue())); + + // Got a stray message -- explicitly reset the association (force remote endpoint to reassociate) + default: + if (_log.IsDebugEnabled) + _log.Debug("Sending disassociate to [{0}] because unexpected message of type [{1}] was received unassociated.", wrappedHandle, @event.FsmEvent.GetType()); + SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); + return Stop(); } + } case HandshakeTimer t when @event.StateData is OutboundUnderlyingAssociated oua: if (_log.IsDebugEnabled) @@ -979,6 +982,12 @@ private void InitializeFSM() SendDisassociate(oua.WrappedHandle, DisassociateInfo.Unknown); return Stop(new Failure(new TimeoutReason( $"No response from remote for outbound association. Handshake timed out after [{_settings.HandshakeTimeout.TotalMilliseconds}] ms"))); + case HandshakeTimer t when @event.StateData is InboundUnassociated iu: + if (_log.IsDebugEnabled) + _log.Debug("Sending disassociate to [{0}] because handshake timed out for inbound association after [{1}] ms.", iu.WrappedHandle, _settings.HandshakeTimeout.TotalMilliseconds); + SendDisassociate(iu.WrappedHandle, DisassociateInfo.Unknown); + return Stop(new Failure(new TimeoutReason( + $"No response from remote for inbound association. Handshake timed out after [{_settings.HandshakeTimeout.TotalMilliseconds}] ms"))); case UnderlyingTransportError ue: PublishError(ue); return Stay(); @@ -989,6 +998,7 @@ private void InitializeFSM() When(AssociationState.Open, @event => { + _log.Info("{0} - {1}: {2}", StateName, StateData, @event.FsmEvent); switch (@event.FsmEvent) { case Disassociated d: @@ -1004,6 +1014,7 @@ private void InitializeFSM() _failureDetector.HeartBeat(); return Stay(); case Payload p: + // use incoming ordinary message as alive sign _failureDetector.HeartBeat(); switch (@event.StateData) { @@ -1019,7 +1030,7 @@ private void InitializeFSM() return Stay(); default: throw new AkkaProtocolException( - $"Unhandled message in state Open(InboundPayload) with type [{@event.FsmEvent.GetType()}]"); + $"Unhandled message in state Open(InboundPayload) with type [{@event.FsmEvent?.GetType()}]"); } default: return Stay(); From 1731bb5d4ee9c3f8a89071723307cb72969b711b Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 26 Apr 2019 11:50:26 -0500 Subject: [PATCH 29/36] may have found the source of the leak --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 3 +- .../Transport/AkkaProtocolTransport.cs | 85 ++++++++++++------- 2 files changed, 56 insertions(+), 32 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index a4014489b77..33bc7b71164 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -751,7 +751,8 @@ private void VerifySend(object msg, Action afterSend) { MuteSystem(Sys); _remoteSystem.EventStream.Publish(EventFilter.Error(start: "AssociationError").Mute()); - _remoteSystem.EventStream.Publish(EventFilter.Exception().Mute()); + // OversizedPayloadException inherits from EndpointException, so have to mute it for now + //_remoteSystem.EventStream.Publish(EventFilter.Exception().Mute()); } private void MuteSystem(ActorSystem system) diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index 16848c22f2d..11116996eab 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -947,34 +947,39 @@ private void InitializeFSM() // Events for inbound associations case InboundPayload p when @event.StateData is InboundUnassociated iu: - { - var pdu = DecodePdu(p.Payload); - /* - * This state is used by inbound protocol state actors - * when they receive an association attempt from the - * outbound side of the association. - */ - var associationHandler = iu.AssociationEventListener; - var wrappedHandle = iu.WrappedHandle; - switch (pdu) { - case Associate a: - SendAssociate(wrappedHandle, _localHandshakeInfo); - _failureDetector.HeartBeat(); - CancelTimer(HandshakeTimerKey); - return GoTo(AssociationState.Open).Using( - new AssociatedWaitHandler( - NotifyInboundHandler(wrappedHandle, a.Info, associationHandler), - wrappedHandle, new Queue())); - - // Got a stray message -- explicitly reset the association (force remote endpoint to reassociate) - default: - if (_log.IsDebugEnabled) - _log.Debug("Sending disassociate to [{0}] because unexpected message of type [{1}] was received unassociated.", wrappedHandle, @event.FsmEvent.GetType()); - SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); - return Stop(); + var pdu = DecodePdu(p.Payload); + /* + * This state is used by inbound protocol state actors + * when they receive an association attempt from the + * outbound side of the association. + */ + var associationHandler = iu.AssociationEventListener; + var wrappedHandle = iu.WrappedHandle; + switch (pdu) + { + case Disassociate d: + // After receiving Disassociate we MUST NOT send back a Disassociate (loop) + return Stop(new Failure(d.Reason)); + case Associate a: + // Incoming association -- implicitly ACK by a heartbeat + SendAssociate(wrappedHandle, _localHandshakeInfo); + _failureDetector.HeartBeat(); + InitHeartbeatTimer(); + CancelTimer(HandshakeTimerKey); + return GoTo(AssociationState.Open).Using( + new AssociatedWaitHandler( + NotifyInboundHandler(wrappedHandle, a.Info, associationHandler), + wrappedHandle, new Queue())); + + // Got a stray message -- explicitly reset the association (force remote endpoint to reassociate) + default: + if (_log.IsDebugEnabled) + _log.Debug("Sending disassociate to [{0}] because unexpected message of type [{1}] was received unassociated.", wrappedHandle, @event.FsmEvent.GetType()); + SendDisassociate(wrappedHandle, DisassociateInfo.Unknown); + return Stop(); + } } - } case HandshakeTimer t when @event.StateData is OutboundUnderlyingAssociated oua: if (_log.IsDebugEnabled) @@ -1104,10 +1109,12 @@ AssociationHandle GetHandle(ProtocolStateData data) "Transport disassociated before handshake finished")); oua.StatusCompletionSource.TrySetException(associationFailure); - oua.WrappedHandle.Disassociate(); + oua.WrappedHandle.Disassociate(DisassociationReason(@event.Reason), _log); }) .With(awh => { + // Invalidate exposed but still unfinished promise. The underlying association disappeared, so after + // registration immediately signal a disassociate Disassociated disassociateNotification = null; if (@event.Reason is Failure && @event.Reason.AsInstanceOf().Cause is DisassociateInfo) { @@ -1120,24 +1127,25 @@ AssociationHandle GetHandle(ProtocolStateData data) } awh.HandlerListener.ContinueWith(result => result.Result.Notify(disassociateNotification), TaskContinuationOptions.ExecuteSynchronously); + awh.WrappedHandle.Disassociate(DisassociationReason(@event.Reason), _log); }) .With(lr => { Disassociated disassociateNotification = null; - if (@event.Reason is Failure && ((Failure)@event.Reason).Cause is DisassociateInfo) + if (@event.Reason is Failure failure && failure.Cause is DisassociateInfo) { disassociateNotification = - new Disassociated(((Failure)@event.Reason).Cause.AsInstanceOf()); + new Disassociated(failure.Cause.AsInstanceOf()); } else { disassociateNotification = new Disassociated(DisassociateInfo.Unknown); } lr.Listener.Notify(disassociateNotification); - lr.WrappedHandle.Disassociate(); + lr.WrappedHandle.Disassociate(DisassociationReason(@event.Reason), _log); }) .With(iu => - iu.WrappedHandle.Disassociate())); + iu.WrappedHandle.Disassociate(DisassociationReason(@event.Reason), _log))); /* * Set the initial ProtocolStateActor state to CLOSED if OUTBOUND @@ -1163,6 +1171,21 @@ AssociationHandle GetHandle(ProtocolStateData data) InitHandshakeTimer(); } + private static string DisassociationReason(Reason reason) + { + switch (reason) + { + case Normal n: + return "the ProtocolStateActor was stopped normally"; + case Shutdown s: + return "the ProtocolStateActor was shutdown"; + case Failure f: + return $"the ProtocolStateActor failed: {f.Cause}"; + default: + throw new AkkaProtocolException($"Unrecogized shutdown reason: {reason}"); + } + } + /// /// TBD /// From 74351cdbaa44ed70f68114f72258da753f207caf Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 26 Apr 2019 12:20:24 -0500 Subject: [PATCH 30/36] fixed minor spelling error --- src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index 11116996eab..0ee902b921b 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -1182,7 +1182,7 @@ private static string DisassociationReason(Reason reason) case Failure f: return $"the ProtocolStateActor failed: {f.Cause}"; default: - throw new AkkaProtocolException($"Unrecogized shutdown reason: {reason}"); + throw new AkkaProtocolException($"Unrecognized shutdown reason: {reason}"); } } From a3229047a208ac3e5d89075c3e4e7c0b356856e4 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 26 Apr 2019 12:25:25 -0500 Subject: [PATCH 31/36] removed debug logging --- src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs index 0ee902b921b..7f607c8a890 100644 --- a/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs +++ b/src/core/Akka.Remote/Transport/AkkaProtocolTransport.cs @@ -835,7 +835,6 @@ private void InitializeFSM() { When(AssociationState.Closed, fsmEvent => { - _log.Info("{0} - {1}: {2}", StateName, StateData, @fsmEvent.FsmEvent); switch (fsmEvent.FsmEvent) { case Status.Failure f: @@ -894,7 +893,6 @@ private void InitializeFSM() //Transport layer events for outbound associations When(AssociationState.WaitHandshake, @event => { - _log.Info("{0} - {1}: {2}", StateName, StateData, @event.FsmEvent); switch (@event.FsmEvent) { case Disassociated d: @@ -1003,7 +1001,6 @@ private void InitializeFSM() When(AssociationState.Open, @event => { - _log.Info("{0} - {1}: {2}", StateName, StateData, @event.FsmEvent); switch (@event.FsmEvent) { case Disassociated d: From 7a8643bd00f169bb35f26a0896b15a84c0937f7a Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 26 Apr 2019 17:35:08 -0500 Subject: [PATCH 32/36] added hack to work around association issues --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 4 ++-- src/core/Akka.Remote/Transport/Transport.cs | 2 +- src/core/Akka.Remote/Transport/TransportAdapters.cs | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index 33bc7b71164..de0876a6874 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -517,7 +517,7 @@ public void Stash_inbound_connections_until_UID_is_known_for_pending_outbound() var dummySelection = thisSystem.ActorSelection(ActorPath.Parse(remoteAddress + "/user/noonethere")); dummySelection.Tell("ping", Sys.DeadLetters); - var remoteHandle = remoteTransportProbe.ExpectMsg(); + var remoteHandle = remoteTransportProbe.ExpectMsg(TimeSpan.FromMinutes(4)); remoteHandle.Association.ReadHandlerSource.TrySetResult((IHandleEventListener)(new ActionHandleEventListener(ev => { }))); // Now we initiate an emulated inbound connection to the real system @@ -549,7 +549,7 @@ public void Stash_inbound_connections_until_UID_is_known_for_pending_outbound() // Finish the handshake for the outbound connection - this will unstash the inbound pending connection. remoteHandle.Association.Write(handshakePacket); - inboundHandleProbe.ExpectMsg(); + inboundHandleProbe.ExpectMsg(TimeSpan.FromMinutes(5)); } finally { diff --git a/src/core/Akka.Remote/Transport/Transport.cs b/src/core/Akka.Remote/Transport/Transport.cs index 8143227422c..f64018395c2 100644 --- a/src/core/Akka.Remote/Transport/Transport.cs +++ b/src/core/Akka.Remote/Transport/Transport.cs @@ -392,7 +392,7 @@ protected AssociationHandle(Address localAddress, Address remoteAddress) /// transports may not support it. Remote endpoint of the channel or connection MAY be notified, but this is not /// guaranteed. /// - /// The transport that provides the handle MUST guarantee that could be called arbitrarily many times. + /// The transport that provides the handle MUST guarantee that could be called arbitrarily many times. /// public void Disassociate(string reason, ILoggingAdapter log) { diff --git a/src/core/Akka.Remote/Transport/TransportAdapters.cs b/src/core/Akka.Remote/Transport/TransportAdapters.cs index 05433af9394..321e23f9331 100644 --- a/src/core/Akka.Remote/Transport/TransportAdapters.cs +++ b/src/core/Akka.Remote/Transport/TransportAdapters.cs @@ -161,6 +161,9 @@ public string AugmentScheme(string originalScheme) /// TBD public Address AugmentScheme(Address address) { + // HACK to resolve https://github.com/akkadotnet/akka.net/pull/3764 + if (address.Protocol.StartsWith(AddedSchemeIdentifier)) + return address; var protocol = AugmentScheme(address.Protocol); return address.WithProtocol(protocol); } From d3d1576694d4b07a0fc916b3d88abc04b198bdca Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Mon, 29 Apr 2019 12:19:51 -0500 Subject: [PATCH 33/36] porting over OutboundConnection timeout spec --- .../Transport/AkkaProtocolSpec.cs | 174 ++++++++++-------- 1 file changed, 100 insertions(+), 74 deletions(-) diff --git a/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs b/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs index 8894d71f52b..64308e819e1 100644 --- a/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs +++ b/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs @@ -15,6 +15,7 @@ using Akka.Remote.Transport; using Akka.TestKit; using Akka.Util.Internal; +using FluentAssertions; using Google.Protobuf; using Xunit; using Xunit.Abstractions; @@ -22,20 +23,20 @@ namespace Akka.Remote.Tests.Transport { - + public class AkkaProtocolSpec : AkkaSpec { #region Setup / Config - Address localAddress = new Address("test", "testsystem", "testhost", 1234); - Address localAkkaAddress = new Address("akka.test", "testsystem", "testhost", 1234); + private readonly Address _localAddress = new Address("test", "testsystem", "testhost", 1234); + private readonly Address _localAkkaAddress = new Address("akka.test", "testsystem", "testhost", 1234); - Address remoteAddress = new Address("test", "testsystem2", "testhost2", 1234); - Address remoteAkkaAddress = new Address("akka.test", "testsystem2", "testhost2", 1234); + private readonly Address _remoteAddress = new Address("test", "testsystem2", "testhost2", 1234); + private readonly Address _remoteAkkaAddress = new Address("akka.test", "testsystem2", "testhost2", 1234); private AkkaPduCodec codec; - SerializedMessage testMsg = + private readonly SerializedMessage testMsg = new SerializedMessage { SerializerId = 0, Message = ByteString.CopyFromUtf8("foo") }; private ByteString testEnvelope; @@ -44,18 +45,46 @@ public class AkkaProtocolSpec : AkkaSpec private IHandleEvent testHeartbeat; private IHandleEvent testPayload; private IHandleEvent testDisassociate(DisassociateInfo info) { return new InboundPayload(codec.ConstructDisassociate(info)); } - private IHandleEvent testAssociate(int uid) { return new InboundPayload(codec.ConstructAssociate(new HandshakeInfo(remoteAkkaAddress, uid))); } - private TimeSpan DefaultTimeout { get { return Dilated(TestKitSettings.DefaultTimeout); } } + private IHandleEvent testAssociate(int uid) { return new InboundPayload(codec.ConstructAssociate(new HandshakeInfo(_remoteAkkaAddress, uid))); } + private TimeSpan DefaultTimeout => Dilated(TestKitSettings.DefaultTimeout); public AkkaProtocolSpec(ITestOutputHelper helper) - : base(@"akka.test.default-timeout = 1.5 s", helper) + : base(@" + akka.actor.provider = remote + akka.test.default-timeout = 1.5 s", helper) { codec = new AkkaPduProtobuffCodec(Sys); - testEnvelope = codec.ConstructMessage(localAkkaAddress, TestActor, testMsg); + testEnvelope = codec.ConstructMessage(_localAkkaAddress, TestActor, testMsg); testMsgPdu = codec.ConstructPayload(testEnvelope); testHeartbeat = new InboundPayload(codec.ConstructHeartbeat()); testPayload = new InboundPayload(testMsgPdu); + + config = ConfigurationFactory.ParseString( + @"akka{ + remote { + + transport-failure-detector { + implementation-class = ""Akka.Remote.PhiAccrualFailureDetector, Akka.Remote"" + threshold = 7.0 + max-sample-size = 100 + min-std-deviation = 100 ms + acceptable-heartbeat-pause = 3 s + heartbeat-interval = 1 s + } + + backoff-interval = 1 s + + require-cookie = off + + secure-cookie = ""abcde"" + + shutdown-timeout = 5 s + + startup-timeout = 5 s + + use-passive-connections = on + }}").WithFallback(Sys.Settings.Config); } public class Collaborators @@ -80,8 +109,8 @@ public Collaborators(AssociationRegistry registry, TestTransport transport, Test public Collaborators GetCollaborators() { var registry = new AssociationRegistry(); - var transport = new TestTransport(localAddress, registry); - var handle = new TestAssociationHandle(localAddress, remoteAddress, transport, true); + var transport = new TestTransport(_localAddress, registry); + var handle = new TestAssociationHandle(_localAddress, _remoteAddress, transport, true); transport.WriteBehavior.PushConstant(true); return new Collaborators(registry, transport, handle, new TestFailureDetector()); } @@ -89,16 +118,10 @@ public Collaborators GetCollaborators() public class TestFailureDetector : FailureDetector { internal volatile bool isAvailable = true; - public override bool IsAvailable - { - get { return isAvailable; } - } + public override bool IsAvailable => isAvailable; internal volatile bool called = false; - public override bool IsMonitoring - { - get { return called; } - } + public override bool IsMonitoring => called; public override void HeartBeat() { @@ -106,31 +129,7 @@ public override void HeartBeat() } } - private Config config = ConfigurationFactory.ParseString( - @"akka{ - remote { - - transport-failure-detector { - implementation-class = ""Akka.Remote.PhiAccrualFailureDetector, Akka.Remote"" - threshold = 7.0 - max-sample-size = 100 - min-std-deviation = 100 ms - acceptable-heartbeat-pause = 3 s - heartbeat-interval = 1 s - } - - backoff-interval = 1 s - - require-cookie = off - - secure-cookie = ""abcde"" - - shutdown-timeout = 5 s - - startup-timeout = 5 s - - use-passive-connections = on - }}"); + private Config config; #endregion @@ -140,7 +139,7 @@ public override void HeartBeat() public void ProtocolStateActor_must_register_itself_as_reader_on_injected_handles() { var collaborators = GetCollaborators(); - Sys.ActorOf(ProtocolStateActor.InboundProps(new HandshakeInfo(localAddress, 42), collaborators.Handle, + Sys.ActorOf(ProtocolStateActor.InboundProps(new HandshakeInfo(_localAddress, 42), collaborators.Handle, new ActorAssociationEventListener(TestActor), new AkkaProtocolSettings(config), codec, collaborators.FailureDetector)); @@ -152,7 +151,7 @@ public void ProtocolStateActor_must_in_inbound_mode_accept_payload_after_Associa { var collaborators = GetCollaborators(); var reader = - Sys.ActorOf(ProtocolStateActor.InboundProps(new HandshakeInfo(localAddress, 42), collaborators.Handle, + Sys.ActorOf(ProtocolStateActor.InboundProps(new HandshakeInfo(_localAddress, 42), collaborators.Handle, new ActorAssociationEventListener(TestActor), new AkkaProtocolSettings(config), codec, collaborators.FailureDetector)); @@ -189,7 +188,7 @@ public void ProtocolStateActor_must_in_inbound_mode_disassociate_when_an_unexpec var collaborators = GetCollaborators(); var reader = - Sys.ActorOf(ProtocolStateActor.InboundProps(new HandshakeInfo(localAddress, 42), collaborators.Handle, + Sys.ActorOf(ProtocolStateActor.InboundProps(new HandshakeInfo(_localAddress, 42), collaborators.Handle, new ActorAssociationEventListener(TestActor), new AkkaProtocolSettings(config), codec, collaborators.FailureDetector)); @@ -214,7 +213,7 @@ public void ProtocolStateActor_must_in_outbound_mode_delay_readiness_until_hands var statusPromise = new TaskCompletionSource(); var reader = - Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(localAddress, 42), remoteAddress, + Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(_localAddress, 42), _remoteAddress, statusPromise, collaborators.Transport, new AkkaProtocolSettings(config), codec, collaborators.FailureDetector)); @@ -233,11 +232,11 @@ public void ProtocolStateActor_must_in_outbound_mode_delay_readiness_until_hands statusPromise.Task.Result.Match() .With(h => { - Assert.Equal(remoteAkkaAddress, h.RemoteAddress); - Assert.Equal(localAkkaAddress, h.LocalAddress); + Assert.Equal(_remoteAkkaAddress, h.RemoteAddress); + Assert.Equal(_localAkkaAddress, h.LocalAddress); Assert.Equal(33, h.HandshakeInfo.Uid); }) - .Default(msg => Assert.True(false,"Did not receive expected AkkaProtocolHandle from handshake")); + .Default(msg => Assert.True(false, "Did not receive expected AkkaProtocolHandle from handshake")); } [Fact] @@ -248,7 +247,7 @@ public void ProtocolStateActor_must_handle_explicit_disassociate_messages() var statusPromise = new TaskCompletionSource(); var reader = - Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(localAddress, 42), remoteAddress, + Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(_localAddress, 42), _remoteAddress, statusPromise, collaborators.Transport, new AkkaProtocolSettings(config), codec, collaborators.FailureDetector)); @@ -260,10 +259,10 @@ public void ProtocolStateActor_must_handle_explicit_disassociate_messages() statusPromise.Task.Result.Match() .With(h => { - Assert.Equal(remoteAkkaAddress, h.RemoteAddress); - Assert.Equal(localAkkaAddress, h.LocalAddress); + Assert.Equal(_remoteAkkaAddress, h.RemoteAddress); + Assert.Equal(_localAkkaAddress, h.LocalAddress); }) - .Default(msg => Assert.True(false,"Did not receive expected AkkaProtocolHandle from handshake")); + .Default(msg => Assert.True(false, "Did not receive expected AkkaProtocolHandle from handshake")); var wrappedHandle = statusPromise.Task.Result.AsInstanceOf(); wrappedHandle.ReadHandlerSource.SetResult(new ActorHandleEventListener(TestActor)); @@ -289,7 +288,7 @@ public void ProtocolStateActor_must_handle_transport_level_disassociations() var statusPromise = new TaskCompletionSource(); var reader = - Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(localAddress, 42), remoteAddress, + Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(_localAddress, 42), _remoteAddress, statusPromise, collaborators.Transport, new AkkaProtocolSettings(config), codec, collaborators.FailureDetector)); @@ -301,10 +300,10 @@ public void ProtocolStateActor_must_handle_transport_level_disassociations() statusPromise.Task.Result.Match() .With(h => { - Assert.Equal(remoteAkkaAddress, h.RemoteAddress); - Assert.Equal(localAkkaAddress, h.LocalAddress); + Assert.Equal(_remoteAkkaAddress, h.RemoteAddress); + Assert.Equal(_localAkkaAddress, h.LocalAddress); }) - .Default(msg => Assert.True(false,"Did not receive expected AkkaProtocolHandle from handshake")); + .Default(msg => Assert.True(false, "Did not receive expected AkkaProtocolHandle from handshake")); var wrappedHandle = statusPromise.Task.Result.AsInstanceOf(); wrappedHandle.ReadHandlerSource.SetResult(new ActorHandleEventListener(TestActor)); @@ -330,7 +329,7 @@ public void ProtocolStateActor_must_disassociate_when_failure_detector_signals_f var statusPromise = new TaskCompletionSource(); var stateActor = - Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(localAddress, 42), remoteAddress, + Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(_localAddress, 42), _remoteAddress, statusPromise, collaborators.Transport, new AkkaProtocolSettings(config), codec, collaborators.FailureDetector)); @@ -342,8 +341,8 @@ public void ProtocolStateActor_must_disassociate_when_failure_detector_signals_f statusPromise.Task.Result.Match() .With(h => { - Assert.Equal(remoteAkkaAddress, h.RemoteAddress); - Assert.Equal(localAkkaAddress, h.LocalAddress); + Assert.Equal(_remoteAkkaAddress, h.RemoteAddress); + Assert.Equal(_localAkkaAddress, h.LocalAddress); }) .Default(msg => Assert.True(false, "Did not receive expected AkkaProtocolHandle from handshake")); var wrappedHandle = statusPromise.Task.Result.AsInstanceOf(); @@ -374,7 +373,7 @@ public void ProtocolStateActor_must_handle_correctly_when_the_handler_is_registe var statusPromise = new TaskCompletionSource(); var stateActor = - Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(localAddress, 42), remoteAddress, + Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(_localAddress, 42), _remoteAddress, statusPromise, collaborators.Transport, new AkkaProtocolSettings(config), codec, collaborators.FailureDetector)); @@ -386,10 +385,10 @@ public void ProtocolStateActor_must_handle_correctly_when_the_handler_is_registe statusPromise.Task.Result.Match() .With(h => { - Assert.Equal(remoteAkkaAddress, h.RemoteAddress); - Assert.Equal(localAkkaAddress, h.LocalAddress); + Assert.Equal(_remoteAkkaAddress, h.RemoteAddress); + Assert.Equal(_localAkkaAddress, h.LocalAddress); }) - .Default(msg => Assert.True(false,"Did not receive expected AkkaProtocolHandle from handshake")); + .Default(msg => Assert.True(false, "Did not receive expected AkkaProtocolHandle from handshake")); var wrappedHandle = statusPromise.Task.Result.AsInstanceOf(); stateActor.Tell(new Disassociated(DisassociateInfo.Unknown), Self); @@ -408,6 +407,33 @@ public void ProtocolStateActor_must_handle_correctly_when_the_handler_is_registe }); } + [Fact] + public void ProtocolStateActor_must_give_up_outbound_after_connection_timeout() + { + var collaborators = GetCollaborators(); + collaborators.Handle.Writeable = false; + collaborators.Transport.AssociateBehavior.PushConstant(collaborators.Handle); + + var statusPromise = new TaskCompletionSource(); + + var conf2 = ConfigurationFactory.ParseString("akka.remote.dot-netty.tcp.connection-timeout = 500 ms") + .WithFallback(config); + + var stateActor = + Sys.ActorOf(ProtocolStateActor.OutboundProps(new HandshakeInfo(_localAddress, 42), _remoteAddress, + statusPromise, collaborators.Transport, + new AkkaProtocolSettings(conf2), codec, collaborators.FailureDetector)); + + Watch(stateActor); + + // inner exception will be a TimeoutException + Intercept(() => + { + statusPromise.Task.Wait(TimeSpan.FromSeconds(5)).Should().BeTrue(); + }); + ExpectTerminated(stateActor); + } + #endregion #region Internal helper methods @@ -418,8 +444,8 @@ private bool LastActivityIsHeartbeat(AssociationRegistry associationRegistry) var rValue = false; if (associationRegistry.LogSnapshot().Last() is WriteAttempt) { - var attempt = (WriteAttempt) associationRegistry.LogSnapshot().Last(); - if (attempt.Sender.Equals(localAddress) && attempt.Recipient.Equals(remoteAddress)) + var attempt = (WriteAttempt)associationRegistry.LogSnapshot().Last(); + if (attempt.Sender.Equals(_localAddress) && attempt.Recipient.Equals(_remoteAddress)) { codec.DecodePdu(attempt.Payload) .Match() @@ -437,12 +463,12 @@ private bool LastActivityIsAssociate(AssociationRegistry associationRegistry, lo var rValue = false; if (associationRegistry.LogSnapshot().Last() is WriteAttempt) { - var attempt = (WriteAttempt) associationRegistry.LogSnapshot().Last(); - if (attempt.Sender.Equals(localAddress) && attempt.Recipient.Equals(remoteAddress)) + var attempt = (WriteAttempt)associationRegistry.LogSnapshot().Last(); + if (attempt.Sender.Equals(_localAddress) && attempt.Recipient.Equals(_remoteAddress)) { codec.DecodePdu(attempt.Payload) .Match() - .With(h => rValue = h.Info.Origin.Equals(localAddress) && h.Info.Uid == uid) + .With(h => rValue = h.Info.Origin.Equals(_localAddress) && h.Info.Uid == uid) .Default(msg => rValue = false); } } @@ -456,8 +482,8 @@ private bool LastActivityIsDisassociate(AssociationRegistry associationRegistry) var rValue = false; if (associationRegistry.LogSnapshot().Last() is WriteAttempt) { - var attempt = (WriteAttempt) associationRegistry.LogSnapshot().Last(); - if (attempt.Sender.Equals(localAddress) && attempt.Recipient.Equals(remoteAddress)) + var attempt = (WriteAttempt)associationRegistry.LogSnapshot().Last(); + if (attempt.Sender.Equals(_localAddress) && attempt.Recipient.Equals(_remoteAddress)) codec.DecodePdu(attempt.Payload) .Match() .With(h => rValue = true) From 26d66830a383eee7912b059a215730a20d9adb5f Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Mon, 29 Apr 2019 12:23:51 -0500 Subject: [PATCH 34/36] ported InboundTimeout spec --- .../Transport/AkkaProtocolSpec.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs b/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs index 64308e819e1..9c33972cced 100644 --- a/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs +++ b/src/core/Akka.Remote.Tests/Transport/AkkaProtocolSpec.cs @@ -434,6 +434,25 @@ public void ProtocolStateActor_must_give_up_outbound_after_connection_timeout() ExpectTerminated(stateActor); } + [Fact] + public void ProtocolStateActor_must_give_up_inbound_after_connection_timeout() + { + var collaborators = GetCollaborators(); + collaborators.Handle.Writeable = false; + collaborators.Transport.AssociateBehavior.PushConstant(collaborators.Handle); + + var conf2 = ConfigurationFactory.ParseString("akka.remote.dot-netty.tcp.connection-timeout = 500 ms") + .WithFallback(config); + + var reader = + Sys.ActorOf(ProtocolStateActor.InboundProps(new HandshakeInfo(_localAddress, 42), collaborators.Handle, + new ActorAssociationEventListener(TestActor), new AkkaProtocolSettings(conf2), codec, + collaborators.FailureDetector)); + + Watch(reader); + ExpectTerminated(reader); + } + #endregion #region Internal helper methods From 32de3cd1d9aa0f32c20d076d67966a721b933af0 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Mon, 29 Apr 2019 12:28:57 -0500 Subject: [PATCH 35/36] approved new Akka.Remote API additions --- .../Akka.API.Tests/CoreAPISpec.ApproveRemote.approved.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core/Akka.API.Tests/CoreAPISpec.ApproveRemote.approved.txt b/src/core/Akka.API.Tests/CoreAPISpec.ApproveRemote.approved.txt index f71f2a367ae..22408b0fc21 100644 --- a/src/core/Akka.API.Tests/CoreAPISpec.ApproveRemote.approved.txt +++ b/src/core/Akka.API.Tests/CoreAPISpec.ApproveRemote.approved.txt @@ -28,6 +28,7 @@ namespace Akka.Remote public class AkkaProtocolSettings { public AkkaProtocolSettings(Akka.Configuration.Config config) { } + public System.TimeSpan HandshakeTimeout { get; } public Akka.Configuration.Config TransportFailureDetectorConfig { get; } public string TransportFailureDetectorImplementationClass { get; } public System.TimeSpan TransportHeartBeatInterval { get; } @@ -509,7 +510,10 @@ namespace Akka.Remote.Transport public Akka.Actor.Address LocalAddress { get; set; } public System.Threading.Tasks.TaskCompletionSource ReadHandlerSource { get; set; } public Akka.Actor.Address RemoteAddress { get; set; } + [System.ObsoleteAttribute("Use the method that states reasons to make sure disassociation reasons are logged" + + ".")] public abstract void Disassociate(); + public void Disassociate(string reason, Akka.Event.ILoggingAdapter log) { } public override bool Equals(object obj) { } protected bool Equals(Akka.Remote.Transport.AssociationHandle other) { } public override int GetHashCode() { } From 60e84b57061a3da11132389e093caaf1f9595b50 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Mon, 29 Apr 2019 14:23:11 -0500 Subject: [PATCH 36/36] removed TransportAdapter hack and cleaned up RemotingSpecs --- src/core/Akka.Remote.Tests/RemotingSpec.cs | 18 +++--------------- .../Akka.Remote/Transport/TransportAdapters.cs | 3 --- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/core/Akka.Remote.Tests/RemotingSpec.cs b/src/core/Akka.Remote.Tests/RemotingSpec.cs index de0876a6874..0208619795b 100644 --- a/src/core/Akka.Remote.Tests/RemotingSpec.cs +++ b/src/core/Akka.Remote.Tests/RemotingSpec.cs @@ -58,13 +58,6 @@ private static string GetConfig() akka { actor.provider = remote - loglevel = DEBUG - - actor.debug{ - unhandled = on - fsm = on - receive = on - } remote { transport = ""Akka.Remote.Remoting,Akka.Remote"" @@ -111,12 +104,7 @@ protected string GetOtherRemoteSysConfig() akka { actor.provider = remote - loglevel = DEBUG - actor.debug{ - unhandled = on - fsm = on - receive = on - } + remote { transport = ""Akka.Remote.Remoting,Akka.Remote"" @@ -534,7 +522,7 @@ public void Stash_inbound_connections_until_UID_is_known_for_pending_outbound() var pduCodec = new AkkaPduProtobuffCodec(Sys); - var handshakePacket = pduCodec.ConstructAssociate(new HandshakeInfo(remoteAddress, 0)); + var handshakePacket = pduCodec.ConstructAssociate(new HandshakeInfo(rawRemoteAddress, 0)); var brokenPacket = pduCodec.ConstructPayload(ByteString.CopyFrom(0, 1, 2, 3, 4, 5, 6)); // Finish the inbound handshake so now it is handed up to Remoting @@ -616,7 +604,7 @@ public void Properly_quarantine_stashed_inbound_connections() var pduCodec = new AkkaPduProtobuffCodec(Sys); - var handshakePacket = pduCodec.ConstructAssociate(new HandshakeInfo(remoteAddress, remoteUID)); + var handshakePacket = pduCodec.ConstructAssociate(new HandshakeInfo(rawRemoteAddress, remoteUID)); // Finish the inbound handshake so now it is handed up to Remoting inboundHandle.Write(handshakePacket); diff --git a/src/core/Akka.Remote/Transport/TransportAdapters.cs b/src/core/Akka.Remote/Transport/TransportAdapters.cs index 321e23f9331..05433af9394 100644 --- a/src/core/Akka.Remote/Transport/TransportAdapters.cs +++ b/src/core/Akka.Remote/Transport/TransportAdapters.cs @@ -161,9 +161,6 @@ public string AugmentScheme(string originalScheme) /// TBD public Address AugmentScheme(Address address) { - // HACK to resolve https://github.com/akkadotnet/akka.net/pull/3764 - if (address.Protocol.StartsWith(AddedSchemeIdentifier)) - return address; var protocol = AugmentScheme(address.Protocol); return address.WithProtocol(protocol); }