Akka.Remote: ensure `RemoteActorRef` are serialized with correct `Address` when using multiple transports #7393

Aaronontheweb · 2024-11-18T19:16:27Z

Changes

Fixes #7378

Checklist

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

This change follows the Akka.NET API Compatibility Guidelines.
This change follows the Akka.NET Wire Compatibility Guidelines.
I have reviewed my own pull request.
Design discussion issue The first of multiple enabled Transports is used #7378
Changes in public API reviewed, if any.

Need to ensure we use `RemoteActorRef` instead of `ActorSelection` in order to properly reproduce bug

Aaronontheweb · 2024-11-18T19:16:56Z

src/core/Akka.Remote.Tests/Transport/MultiTransportAddressingSpec.cs

+            var actor = await selection.ResolveOne(TimeSpan.FromSeconds(1));
+
+            // assert that the remote actor is using the correct transport
+            Assert.Contains(scheme, actor.Path.Address.Protocol);


Currently fails here - the IActorRef returned to the ResolveOne method belongs to transport 1 rather than transport 2.

Aaronontheweb · 2024-11-18T19:20:21Z

From the perspective of node 2, the Sender from node 1 has the correct address (given that it's using an ActorSelection to deliver the Identify here)

Aaronontheweb · 2024-11-18T19:49:31Z

So I thought this might be the culprit

akka.net/src/core/Akka.Remote/Transport/AkkaPduCodec.cs

Lines 556 to 564 in 5e2bd0e

    
           private static ActorRefData SerializeActorRef(Address defaultAddress, IActorRef actorRef) 
        
           { 
        
               return new ActorRefData() 
        
               { 
        
                   Path = (!string.IsNullOrEmpty(actorRef.Path.Address.Host)) 
        
                       ? actorRef.Path.ToSerializationFormat() 
        
                       : actorRef.Path.ToSerializationFormatWithAddress(defaultAddress) 
        
               }; 
        
           }

But no, that code correctly serializes the Sender from node2 using Transport2's address - so this means the problem is likely on the read-side instead.

close akkadotnet#7378

Aaronontheweb

Detailed my changes - I believe we have fixed the issue (but we'll need to check the results of the full Akka.Remote and up test suite first to confirm)

Aaronontheweb · 2024-11-18T20:40:42Z

src/core/Akka.Remote/EndpointManager.cs

Made some minor formatting changes here - nothing substantial

Aaronontheweb · 2024-11-18T20:48:10Z

src/core/Akka.Remote/Endpoint.cs

@@ -1357,7 +1359,7 @@ private SerializedMessage SerializeMessage(object msg)
            {
                throw new EndpointException("Internal error: No handle was present during serialization of outbound message.");
            }
-            return MessageSerializer.Serialize(_system, _handle.LocalAddress, msg);
+            return MessageSerializer.Serialize(_system, _transportInformation, msg);


This is part of the real fix - make sure we're always setting the current transport information to our transport before we perform outbound serialization. That way, any messages included inside the payloads get serialized onto the correct transport when we have multiple running.

Aaronontheweb · 2024-11-18T20:48:49Z

src/core/Akka.Remote/MessageSerializer.cs

-                if (oldInfo == null)
-                    Akka.Serialization.Serialization.CurrentTransportInformation =
-                        system.Provider.SerializationInformation;
+                Akka.Serialization.Serialization.CurrentTransportInformation = transportInformation;


The other part of the real fix - make sure we explicitly set the CurrentTransportInformation to whatever was passed in by the EndpointReader before we get going

Aaronontheweb · 2024-11-18T20:56:31Z

Performance Numbers

Running on .NET 8

`dev`

Run 1

Attempted to elevate process priority, but failed due to Permission denied - carrying on at normal process priority.
OSVersion:                         Unix 6.9.3.76060903
ProcessorCount:                    16
ClockSpeed:                        0 MHZ
Actor Count:                       32
Messages sent/received per client: 200000  (2e5)
Is Server GC:                      True
Thread count:                      43

Num clients, Total [msg], Msgs/sec, Total [ms], Start Threads, End Threads
         1,  200000,    106383,    1880.72,            43,              68
         5, 1000000,    374392,    2671.14,            76,              85
        10, 2000000,    701755,    2850.07,            93,              93
        15, 3000000,    732601,    4095.40,           101,             101
        20, 4000000,    747524,    5351.44,           109,             102
        25, 5000000,    780397,    6407.86,           110,              94
        30, 6000000,    805045,    7453.31,           102,              93

Run 2

Attempted to elevate process priority, but failed due to Permission denied - carrying on at normal process priority.
OSVersion:                         Unix 6.9.3.76060903
ProcessorCount:                    16
ClockSpeed:                        0 MHZ
Actor Count:                       32
Messages sent/received per client: 200000  (2e5)
Is Server GC:                      True
Thread count:                      42

Num clients, Total [msg], Msgs/sec, Total [ms], Start Threads, End Threads
         1,  200000,    119403,    1675.34,            42,              69
         5, 1000000,    316056,    3164.79,            77,              82
        10, 2000000,    752729,    2657.33,            90,              91
        15, 3000000,    772997,    3881.94,            99,             103
        20, 4000000,    789422,    5067.17,           111,             111
        25, 5000000,    823859,    6069.37,           119,              95
        30, 6000000,    815661,    7356.32,           103,              95

Run 3

Attempted to elevate process priority, but failed due to Permission denied - carrying on at normal process priority.
OSVersion:                         Unix 6.9.3.76060903
ProcessorCount:                    16
ClockSpeed:                        0 MHZ
Actor Count:                       32
Messages sent/received per client: 200000  (2e5)
Is Server GC:                      True
Thread count:                      42

Num clients, Total [msg], Msgs/sec, Total [ms], Start Threads, End Threads
         1,  200000,    114614,    1745.04,            42,              70
         5, 1000000,    418061,    2392.65,            78,              83
        10, 2000000,    752729,    2657.07,            91,              91
        15, 3000000,    783700,    3828.12,            99,              99
        20, 4000000,    785238,    5094.91,           107,             107
        25, 5000000,    801283,    6240.87,           115,              92
        30, 6000000,    795545,    7542.17,           100,              92
Done..

This PR

Run 1

Attempted to elevate process priority, but failed due to Permission denied - carrying on at normal process priority.
OSVersion:                         Unix 6.9.3.76060903
ProcessorCount:                    16
ClockSpeed:                        0 MHZ
Actor Count:                       32
Messages sent/received per client: 200000  (2e5)
Is Server GC:                      True
Thread count:                      42

Num clients, Total [msg], Msgs/sec, Total [ms], Start Threads, End Threads
         1,  200000,    108933,    1836.03,            42,              68
         5, 1000000,    392619,    2547.06,            76,              80
        10, 2000000,    722544,    2768.23,            88,              89
        15, 3000000,    739828,    4055.51,            97,             101
        20, 4000000,    756144,    5290.07,           109,             109
        25, 5000000,    758266,    6594.39,           117,              94
        30, 6000000,    763748,    7856.72,           102,              94

Run 2

ttempted to elevate process priority, but failed due to Permission denied - carrying on at normal process priority.
OSVersion:                         Unix 6.9.3.76060903
ProcessorCount:                    16
ClockSpeed:                        0 MHZ
Actor Count:                       32
Messages sent/received per client: 200000  (2e5)
Is Server GC:                      True
Thread count:                      42

Num clients, Total [msg], Msgs/sec, Total [ms], Start Threads, End Threads
         1,  200000,    111112,    1800.84,            42,              69
         5, 1000000,    381389,    2622.41,            77,              82
        10, 2000000,    734485,    2723.75,            90,              91
        15, 3000000,    762583,    3934.46,            99,              99
        20, 4000000,    766284,    5220.71,           107,             108
        25, 5000000,    787774,    6347.51,           116,              98
        30, 6000000,    775294,    7739.02,           106,              98

Run 3

Attempted to elevate process priority, but failed due to Permission denied - carrying on at normal process priority.
OSVersion:                         Unix 6.9.3.76060903
ProcessorCount:                    16
ClockSpeed:                        0 MHZ
Actor Count:                       32
Messages sent/received per client: 200000  (2e5)
Is Server GC:                      True
Thread count:                      43

Num clients, Total [msg], Msgs/sec, Total [ms], Start Threads, End Threads
         1,  200000,    112740,    1774.65,            43,              70
         5, 1000000,    385357,    2595.37,            78,              83
        10, 2000000,    738826,    2707.74,            91,              91
        15, 3000000,    758342,    3956.67,            99,             100
        20, 4000000,    759302,    5268.95,           108,             108
        25, 5000000,    785300,    6367.31,           116,              92
        30, 6000000,    786164,    7632.28,           100,              92

No real performance impact, which is good 👍

Arkatufus

LGTM

harden specs for akkadotnet#7378

0d4a946

Need to ensure we use `RemoteActorRef` instead of `ActorSelection` in order to properly reproduce bug

Aaronontheweb added akka-remote serialization labels Nov 18, 2024

Aaronontheweb commented Nov 18, 2024

View reviewed changes

changed up ActorSystem names to make it easier to debug

f5a437b

fixed outbound address serialization

13f91b5

close akkadotnet#7378

Aaronontheweb changed the title ~~[WIP] ensure RemoteActorRef are serialized with correct Address when using multiple transports~~ Akka.Remote: ensure RemoteActorRef are serialized with correct Address when using multiple transports Nov 18, 2024

fixed compilation issue

0cfcbde

Aaronontheweb commented Nov 18, 2024

View reviewed changes

Aaronontheweb mentioned this pull request Nov 18, 2024

The first of multiple enabled Transports is used #7378

Closed

Merge branch 'dev' into fix-7378-transport-specs

9cac419

Arkatufus approved these changes Nov 19, 2024

View reviewed changes

Arkatufus enabled auto-merge (squash) November 19, 2024 15:24

Arkatufus merged commit 4ae4792 into akkadotnet:dev Nov 19, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Akka.Remote: ensure `RemoteActorRef` are serialized with correct `Address` when using multiple transports #7393

Akka.Remote: ensure `RemoteActorRef` are serialized with correct `Address` when using multiple transports #7393

Aaronontheweb commented Nov 18, 2024 •

edited

Loading

Aaronontheweb Nov 18, 2024

Aaronontheweb commented Nov 18, 2024

Aaronontheweb commented Nov 18, 2024

Aaronontheweb left a comment

Aaronontheweb Nov 18, 2024

Aaronontheweb Nov 18, 2024

Aaronontheweb Nov 18, 2024

Aaronontheweb commented Nov 18, 2024

Arkatufus left a comment

Akka.Remote: ensure RemoteActorRef are serialized with correct Address when using multiple transports #7393

Akka.Remote: ensure RemoteActorRef are serialized with correct Address when using multiple transports #7393

Conversation

Aaronontheweb commented Nov 18, 2024 • edited Loading

Changes

Checklist

Aaronontheweb Nov 18, 2024

Choose a reason for hiding this comment

Aaronontheweb commented Nov 18, 2024

Aaronontheweb commented Nov 18, 2024

Aaronontheweb left a comment

Choose a reason for hiding this comment

Aaronontheweb Nov 18, 2024

Choose a reason for hiding this comment

Aaronontheweb Nov 18, 2024

Choose a reason for hiding this comment

Aaronontheweb Nov 18, 2024

Choose a reason for hiding this comment

Aaronontheweb commented Nov 18, 2024

Performance Numbers

dev

This PR

Arkatufus left a comment

Choose a reason for hiding this comment

Akka.Remote: ensure `RemoteActorRef` are serialized with correct `Address` when using multiple transports #7393

Akka.Remote: ensure `RemoteActorRef` are serialized with correct `Address` when using multiple transports #7393

Aaronontheweb commented Nov 18, 2024 •

edited

Loading

`dev`