Remote Association creation revamp, removal of hack-workaround in RemoteRef #577

ktoso · 2020-04-22T04:03:53Z

Motivation:

We did not solve for some situations how an association may be needed.

E.g. you get a ref to node C from node B, but you never knew node C yet. If you started sending messages to that ref, before the other parts of the system ensured the association you could end up sending them into the void causing message loss.

Modifications:

the association is eager so we store it as we initiate the handshake dance, not only as the result
this gives us a place to enqueue / buffer the messages to be sent

Result:

More resilient message sends, no message loss
Resolves OperationLog replication / OpLogReceptionist should use ProtoBuf #482 OperationLog replication / OpLogReceptionist should use ProtoBuf
Resolves Test and harden: ActorRef+RemotePersonality.remoteControl optimizations #382 Test and harden: ActorRef+RemotePersonality.remoteControl optimizations
Resolves FAILED: Occasional: Exited with signal code 11 #492 FAILED: Occasional: Exited with signal code 11
Resolves FAILED: test_singletonByClusterLeadership_withLeaderChange #435 Singleton tests which uncovered races in associations before
Resolves FAILED: test_singletonByClusterLeadership_stashMessagesIfNoLeader; "handshake which was not in progress!" #486 FAILED: test_singletonByClusterLeadership_stashMessagesIfNoLeader; "handshake which was not in progress!"
Resolves FAILED: ActorSingletonPluginClusteredTests.test_singletonByClusterLeadership #463 FAILED: ActorSingletonPluginClusteredTests.test_singletonByClusterLeadership
Introduced and Resolves FAILED: only once through test_swim_shouldNotifyClusterAboutUnreachableNode_whenUnreachableDiscoveredByOtherNode #597 😉
opens up posibility for more "proper" ActorTransport -- it will be behind the association

ktoso · 2020-04-24T07:58:28Z

Trying to figure out why test_ensureDownAndRemovalSpreadsToAllMembers fails more often now... It should not have impacted SWIM much really 🤔

ktoso · 2020-04-24T10:32:04Z

Follow up for future: Reduce locking or lockless Associations #579

ktoso · 2020-04-24T10:37:30Z

Facing an always reproducing ClusterLeaderActionsClusteredTests.test_ensureDownAndRemovalSpreadsToAllMembers now but can't find why that is since a few hours... Not moving into suspect, but that's SWIM things which are really unchanged by this PR yet it reliaby fails here and is fine on master 🤔

yim-lee · 2020-04-24T17:54:32Z

Is the failing test making the right assumptions/assertions? 🤔

ktoso · 2020-04-27T02:45:16Z

Sadly it seems it has the right assumptions hm hm, digging more.

ktoso · 2020-04-27T10:49:26Z

Seems I'm not used to the revised SWIM behavior after introduction of the LHA modifications :) It can take a while to trigger the down now in small clusters. That's fair and fine to be honest. Triggering those is better safe than sorry. Tweaked some things and some minor cleanups to make logs less verbose.

ktoso · 2020-04-27T12:12:10Z

~~Huh an integration test failure... probably timing there, checking~~ A place that used localhost instead of 127.0.0.1

ktoso · 2020-04-27T15:34:09Z

@swift-server-bot test this please

ktoso · 2020-04-27T15:55:32Z

Ok all green here, unlocked another test now :)

yim-lee · 2020-04-27T22:35:54Z

Sources/DistributedActors/Cluster/Transport/RemoteClusterActorPersonality.swift

+/// is clear about its current lifecycle state (it may have already terminated the moment the message was sent,
+/// or even before then). To obtain lifecycle status of this actor the usual strategy of watching it needs to be employed.
+// TODO: reimplement as CellDelegate as it shall become simply another transport?
+public final class RemoteClusterActorPersonality<Message: Codable> {


Can public be removed?

I wish! Sadly that's very tricky :-(

I'll make a ticket to see if we'd be able to make it so. It shows up in the public enum Personality { so it has to be public and in some protocol requirements...

Would be nice if we could make those internal though; but on the other hand, if we need external ActorTransports, we may need to make those not even public but e.g. CellDelegate has to be even open 🤔

To revisit: #591

yim-lee · 2020-04-27T22:38:52Z

Tests/DistributedActorsTests/Cluster/RemotingTLSClusteredTests.swift

        let testKey: NIOSSLPrivateKeySource = .privateKey(try NIOSSLPrivateKey(bytes: [UInt8](testKey1.utf8), format: .pem))
        let local = self.setUpNode("local") { settings in
-            settings.cluster.node.host = "127.0.0.1"
+            settings.cluster.node.host = "localhost"


When should one use 127.0.0.1 vs localhost?

That's a slight annoyance :-( So we compare the address that someone extends a handshake to is "the same" as we bound to. So so if a service bound as localhost, other nodes must join("localhost" ... and the same with 127.0.0.1... We should perhaps add some special handling for this, to be able to treat localhost as 127.0.0.1 when doing this comparison?

New ticket: #590

So the behavior is the same for localhost and 127.0.0.1--basically pick one and stick with it? I guess we just need to document/explain it somewhere. I saw other code comments and thought 127.0.0.1 was preferred (maybe it is) and wondered why we changed to localhost here.

Yeah good point we should "stick to 127.0.0.1", this change here likely was not necessary hm...

ktoso · 2020-04-28T02:52:49Z

And nasty double complete association... debugging

ktoso · 2020-04-28T15:49:55Z

I broke things while trying to get more things passing in stable way... Will have to revisit again.

test_sendingMessageToNotYetAssociatedNode_mustCauseAssociationAttempt now fails which was the entire purpose of this PR, so I messed up big time somewhere since monday :-/ and the Singleton tests manifesting problems...

ktoso · 2020-04-29T09:59:47Z

Wohoo all green out of the "nasty ones" though the PR needs a rebase as it hit

18:48:55 error: Expected String [actor:sact://[email protected]:9001/user/alpha] to contain: [actor:sact://CRDTSerializationTests@localhost:9001/user/alpha] introduced yesterday -- since we stick to 127.0.0.1 by default now

ktoso · 2020-04-30T13:39:16Z

Sources/DistributedActorsTestKit/ActorTestKit.swift

 public struct ActorTestKitSettings {
    /// Timeout used by default by all the `expect...` and `within` functions defined on the testkit and test probes.
-    var expectationTimeout: TimeAmount = .seconds(3)
+    var expectationTimeout: TimeAmount = .seconds(5)


I looked at a test failure on CI just now and they often are "just on the edge of making the timeout" and locally they're fine and more often missing it on CI -- these matter for multi node tests where we're waiting for some random gossip to cause the event etc. They're not entirely predictable, so making the default general timeout I think will be good for us.

If a message comes in quickly there's no problem at all -- an expect immediately gets it and returns after all, so this increase should NOT have impact on build times except hitting those which are right but slow (because non deterministic how/when that message is going to be sent)

In other words, 3 seconds means often that we give a gossip 2 (often not lucky enough to make 3) gossip rounds to get us an information; with 5 we give it more changes -- if the information (most often this is cluster events like down / removed), has a chance to do the gossip dance (convergence is needs a few rounds) and get us the event

ktoso · 2020-04-30T13:40:58Z

Tests/DistributedActorsTests/Cluster/ClusterLeaderActionsClusteredTests.swift

            let eventsOnFirstSub = try p1.expectMessages(count: 9)
+            for event in eventsOnFirstSub {
+                pinfo("Captured event: \(event)")
+            }


to help debugging good vs. bad executions

ktoso · 2020-04-30T13:42:12Z

IntegrationTests/tests_03_xpc_actorable/it_XPCActorable_echo_service/main.swift

 let system = ActorSystem("it_XPCActorable_echo_service") { settings in
    settings.transports += .xpcService

-    settings.cluster.swim.failureDetector.pingTimeout = .seconds(3)


this really is an acknowlagement to how we now use SWIM; it IS our failure detector.

It used to be both the only membership we had and the failure detector.
Now we're more true to how we use it in the settings.

ktoso · 2020-04-30T13:43:17Z

...ationTests/tests_04_cluster/test_01_suspend_causes_unreachable_unsuspend_causes_reachable.sh

 wait_log_exists ${first_logs} 'Binding to: ' 200 # since it might be compiling again...

-stdbuf -i0 -o0 -e0 swift run it_Clustered_swim_suspension_reachability 8228 localhost 7337 > ${second_logs} 2>&1 &
+stdbuf -i0 -o0 -e0 swift run it_Clustered_swim_suspension_reachability 8228 127.0.0.1 7337 > ${second_logs} 2>&1 &


changed defaults everywhere, was getting confused from a mix; now it's 127.0.0.1 everywhere by default.

ktoso · 2020-04-30T13:44:26Z

Protos/WireProtocol.proto

-    UniqueNode from         = 2;
-    Node to                 = 3;
+    UniqueNode originNode   = 2;
+    Node targetNode         = 3;


one of the first things / types I ever did for this project 😉
The naming from/to seemed good at the time, but since then we have a more common local/remote target/origin wording, so keeping those words all the way through

Just curious--How come targetNode is Node instead of UniqueNode?

That's on purpose -- in say a user writes join(Node(127.0.0.1 8888)) we don't know that nodes NodeID and thus it cannot be an UniqueNode, only once the node replies we know it's unique id.

// If we discovered the node using gossip or an actor ref indicating an actor on the remote node we would indeed know the NID though; but the handshake is extended to "some node on that address".

If we end up associating and that NID is different than the uniqueNode used by some actor ref, it means that other unique node is likely "wrong" in the sense that it likely was a previous actor system instance on the same host:port, but it's a new instance/process. Such messages would end up being deadlettered.

I'll add a specific test for this: Test: Hold "old ref" while new associated node on same host:port is associated #603

ktoso · 2020-04-30T13:50:54Z

Samples/Sources/SampleCluster/main.swift

 // end::cluster-sample-actors-discover-and-chat[]

-system.park(atMost: .seconds(60))
+system.park(atMost: .seconds(6000))


basically keep them around "forever" when testing looking at logs ;)

ktoso · 2020-04-30T13:51:04Z

Sources/DistributedActors/ActorShell.swift

-//        where Message: ActorMessage {
-//        self.watch(try self.spawn(naming, props: props, behavior))
-//    }
-


ktoso · 2020-04-30T14:07:29Z

Heh all green 5.3 but some flaky tests remain. I'll not fix them all in this PR, but will continue the fight ⚔️

Was:

FAILED: CRDTActorOwnedTests.test_actorOwned_ORSet_add_many_times #550 CRDTActorOwnedTests.test_actorOwned_ORSet_add_many_times
FAILED: ClusterLeaderActionsClusteredTests.test_down_to_removed_ensureRemovalHappensWhenAllHaveSeenDown FAILED: ClusterLeaderActionsClusteredTests.test_down_to_removed_ensureRemovalHappensWhenAllHaveSeenDown #464

ktoso · 2020-04-30T13:51:27Z

Sources/DistributedActors/Behaviors.swift

+            Message: \(message)
+            Actor: \(context)
+            This is a bug, please open a ticket.
+            """, file: file, line: line)


More details in case this happens again, the previous crash would not be easy to follow up on

ktoso · 2020-04-30T13:55:32Z

Sources/DistributedActors/Cluster/Association.swift


 // ==== ----------------------------------------------------------------------------------------------------------------
-// MARK: Remote Association State Machine
+// MARK: Association


the change™

Associations used to be created only once a handshake completed. So "an association is the result of a handshake" this is wrong and cannot work.

Reasons:

we might be racing handshakes and also various reasons to kick off a handshake

while the handshake is being made, we might be trying to send to the remote already; we MUST have a place to queue up messages rather than dropping them on the floor while the handshake is negotiating

this means the association MUST be created eagerly, and then we always use to it "send" and the send may be a a) real send b) an enqueue or c) the association (the node is down/dead) is a tombstone.

These associations must be accessible without actor messaging and cacheable by remote refs -- this way sending to a remote does nto need a "request/reply" with the actor shell - we just grab the association (and we can cache it (!)). The getting the association today is a normal lock, but we will be able to either RWLock it or go lockless (hopefully... it's technically possible, though somewhat hard -- I'd want to do this with Swift atomics)

ktoso · 2020-04-30T13:56:18Z

Sources/DistributedActors/Cluster/Association.swift

+    }
+
+    /// Complete the association and drain any pending message sends onto the channel.
+    // TODO: This style can only ever work since we lock around the entirety of enqueueing messages and this setting; make it such that we don't need the lock eventually


Reduce locking or lockless Associations #579

ktoso · 2020-04-30T13:57:29Z

Sources/DistributedActors/Cluster/Association.swift

+                }
+
+                // 2) execute any pending tasks and clear them
+                self.runCompletionTasks()


long story how it would be very annoying to have futures in here, since we need to create associations in remote refs and other places -- making this task list is much simpler.

ktoso · 2020-04-30T13:58:15Z

Sources/DistributedActors/Cluster/Association.swift

+    ///
+    /// Tombstones are slightly lighter than a real association, and are kept for a maximum of `settings.cluster.associationTombstoneTTL` TODO: make this setting (!!!)
+    /// before being cleaned up.
+    struct Tombstone: Hashable {


Upcoming work to polish tombstone handling: Cluster: Prune association tombstones #356

ktoso · 2020-04-30T14:11:29Z

Sources/DistributedActors/Cluster/ClusterShell.swift

-            return state
-        }
+        self.terminateAssociation(context.system, state: &state, memberToDown.node)
+        state = self.interpretLeaderActions(context.system, state, state.collectLeaderActions())


leader actions internally checks if we're leader and should do things; less special casing here 👍

ktoso · 2020-04-30T14:11:43Z

Sources/DistributedActors/Cluster/ClusterShellState.swift

    /// This MAY return `inFlight`, in which case it means someone already initiated a handshake with given node,
    /// and we should _do nothing_ and trust that our `whenCompleted` will be notified when the already in-flight handshake completes.
-    mutating func registerHandshake(with remoteNode: Node, whenCompleted: EventLoopPromise<Wire.HandshakeResponse>) -> HandshakeStateMachine.State {
+    mutating func beginHandshake(with remoteNode: Node, whenCompleted: EventLoopPromise<Wire.HandshakeResponse>) -> HandshakeStateMachine.State {


it's not just register, it's kicking it off

ktoso · 2020-04-30T14:16:37Z

Tests/ActorSingletonPluginTests/ActorSingletonPluginClusteredTests.swift

            let replyProbe1 = self.testKit(first).spawnTestProbe(expecting: String.self)
            let ref1 = try first.singleton.host(GreeterSingleton.Message.self, settings: singletonSettings, GreeterSingleton.makeBehavior(instance: GreeterSingleton("Hello-1")))
-            ref1.tell(.greet(name: "Charlie-1", _replyTo: replyProbe1.ref))
+            ref1.tell(.greet(name: "Alice-1", _replyTo: replyProbe1.ref))


The namings here were a bit confusing (the same), so adjusted a bit

ktoso · 2020-04-30T14:16:54Z

Tests/DistributedActorsTests/Cluster/AssociationClusteredTests.swift

        try assertAssociated(remote, withExactly: local.cluster.node)
    }

+    func test_boundServer_shouldAcceptAssociate_raceFromBothNodes() throws {


This would have failed before, now it's solid.

ktoso · 2020-04-30T14:17:17Z

Tests/DistributedActorsTests/Cluster/AssociationClusteredTests.swift

        }
    }

+    func test_sendingMessageToNotYetAssociatedNode_mustCauseAssociationAttempt() throws {


This would have dropped messages and never worked before, now just works 🚀

ktoso · 2020-04-30T15:18:36Z

FAILED TESTS:
  - 3089:Test Case 'ClusterLeaderActionsClusteredTests.test_down_to_removed_ensureRemovalHappensWhenAllHaveSeenDown' failed (3.134 seconds)
  - 3432:Test Case 'ClusterLeaderActionsClusteredTests.test_up_ensureAllSubscribersGetMovingUpEvents' failed (3.06 seconds)
  - 3886:Test Case 'SWIMShellClusteredTests.test_SWIMShell_shouldMonitorJoinedClusterMembers' failed (1.13 seconds)

Hmm weird those are pretty stable locally, I'll dig some more tomorrow before merging.

ktoso · 2020-04-30T15:20:46Z

(Not expecting a full review here, but if you'd want to skim it @yim-lee or @drexin most notably Association, ClusterShell and ClusterShellState that'd be cool, other files I think you can ignore -- i commented on the more crucial parts as well).

yim-lee · 2020-04-30T16:08:40Z

Protos/WireProtocol.proto

-    UniqueNode from         = 2;
-    Node to                 = 3;
+    UniqueNode originNode   = 2;
+    Node targetNode         = 3;


Just curious--How come targetNode is Node instead of UniqueNode?

yim-lee · 2020-04-30T17:38:22Z

Sources/DistributedActors/Cluster/ClusterSettings.swift


    /// If enabled, logs membership changes (including the entire membership table from the perspective of the current node).
-    public var logMembershipChanges: Logger.Level? = .info
+    public var logMembershipChanges: Logger.Level? = .debug


Doesn't .debug lead to more logging though?

Sources/DistributedActors/Cluster/ClusterShell.swift

yim-lee · 2020-04-30T17:46:06Z

Sources/DistributedActors/Cluster/ClusterShellState.swift

+            /// Since we MAY have 2 connections open at this point in time -- one we opened, and another that was opened
+            /// to us when the other node tried to associated, we'll perform a tie-breaker to ensure we predictably
+            /// only use _one_ of them, and close the other.
+            // let selectedChannel: Channel


yim-lee · 2020-04-30T17:47:40Z

Sources/DistributedActors/Cluster/ClusterShellState.swift

+        }
+        if let n = uniqueNode {
+            metadata["handshake/peer"] = "\(n)"
+        }


Is it possible to have both node and uniqueNode be non-nil?

Sources/DistributedActors/Cluster/SWIM/SWIMSettings.swift

yim-lee · 2020-04-30T18:49:29Z

Sources/DistributedActors/Cluster/SystemMessages+Redelivery.swift

        // ./' ./' ./' Never gonna give you up, never gonna let you down ./' ./' ./'
        // we currently never give up

+        // FIXME: implement giving up reconnecting


Related to: Cluster: Prune association tombstones #356
I'll make a new one specifically for this though, thanks! Smaller tickets == better..

Cluster: Implement retrying and giving up in handshakes #604

yim-lee · 2020-04-30T18:55:26Z

Sources/DistributedActors/Cluster/Transport/TransportPipelines.swift

+
+        do {
+            let bytes: ByteBuffer = try proto.serializedByteBuffer(allocator: context.channel.allocator)
+            // TODO: should we use the serialization infra ourselves here? I guess so...


need ticket?

Thanks yes #605

yim-lee · 2020-04-30T19:08:28Z

Tests/DistributedActorsTests/Cluster/AssociationClusteredTests.swift

-        _ = try p7337.expectMessage()
-        _ = try p8228.expectMessage()
+//        _ = try firstProbe.expectMessage()
+//        _ = try secondProbe.expectMessage()


yim-lee · 2020-04-30T19:09:37Z

Tests/DistributedActorsTests/Cluster/AssociationClusteredTests.swift

    // MARK: Joining into existing cluster

-    // FIXME: unlock this test
+    // FIXME: unlock this test // revisit


need this still?

oh no, i forgot to unlock it; thanks. Not passing... :\

…shake

solve the handshake truncation issue; ordering of flushing and sending the handshake we solve this by strongly ordering that flushing messages only happens after we write the handshake reply; as well as signaling the replyTo of the handshake only once we've swapped the association to ready

…ite some leeway

…ck msgs

much confusion and in one of many code paths we get it slightly wrong -- and it's really only used in tests... not entirely worth it -- we will revive when we need it

ktoso · 2020-05-01T11:13:43Z

(Fighting some last timing sensitive tests on CI, adding more debug information / timings to diagnose better)

Oh I see, there's two issues:

the test assumes specific number of events being emitted - that's somewhat error prone, let's never do that
an issue Stress test and fix: Ensure we never emit the same Cluster.Event twice #606 wich an event being sent twice -- we must never do that even though this is not a strong hazard but it can be super confusing, so let's not do it

…ues on CI

ktoso · 2020-05-01T12:33:14Z

Ok I think I've had it with this PR growing too large; we have way less flaky tests but enough remain to be annoying and we must fix them asap as it hinders development -- i'll continue on this when I'm back 🙇

I want to go back to small PRs addressing specific failures.

Now: #608

ktoso changed the title ~~[WIP] Association creation revamp~~ Remote Association creation revamp, removal of hack-workaround in RemoteRef Apr 24, 2020

ktoso mentioned this pull request Apr 27, 2020

FAILED: Occasional: Exited with signal code 11 #492

Closed

yim-lee approved these changes Apr 27, 2020

View reviewed changes

ktoso mentioned this pull request Apr 28, 2020

Special handle localhost/127.0.0.1 in handshakes? #590

Open

ktoso mentioned this pull request Apr 28, 2020

FAILED: test_singletonByClusterLeadership_withLeaderChange #435

Closed

ktoso commented Apr 30, 2020

View reviewed changes

ktoso requested a review from yim-lee April 30, 2020 15:19

ktoso requested a review from drexin April 30, 2020 15:20

yim-lee approved these changes Apr 30, 2020

View reviewed changes

ktoso added 17 commits May 1, 2020 10:41

=remote #482 #382 #383 Race free association / remote sends

d82c40e

=swim log also when suspision comes in from incoming gossip

0195ee8

=handshake no reason to crash, simply reject incoming unexpected hand…

0c66985

…shake

=cluster cleanups, swim logging and timeouts in tests

24c22a0

=int-tests we're using 127.0.0.1 by default now

74e8190

=test unlock test_singletonByClusterLeadership_withLeaderChange

6c1a39f

=remote protect association from being completed multiple times

ab2a941

fix the killing of the "good" connection as well

d268d15

=test,fix default address uses 127.0.0.1 fix test expectation

674b1f2

=test adjust that we use 127.0.0.1 in tests now

f3e5f72

=test,downing increase timeouts as current default detection gives qu…

639cc2a

…ite some leeway

=test increase default expectation timeout; not much pentalty for qui…

805fec7

…ck msgs

wip fixing associations more and more

78b3d4c

INTENSE simplification, drop the whenCompleted future -- it's causing

a26a8a6

much confusion and in one of many code paths we get it slightly wrong -- and it's really only used in tests... not entirely worth it -- we will revive when we need it

=tests need to use 127.0.0.1 rather than localhost for consistency

9e74a11

=test cleanups

423f60b

include time information in pprints so we can easier debug timing iss…

32a5516

…ues on CI

ktoso merged commit 6fb66f4 into apple:master May 1, 2020

ktoso deleted the wip-transport-association-revamp branch May 1, 2020 12:34

Remote Association creation revamp, removal of hack-workaround in RemoteRef #577

Remote Association creation revamp, removal of hack-workaround in RemoteRef #577

Uh oh!

Conversation

ktoso commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation:

Modifications:

Result:

Uh oh!

ktoso commented Apr 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ktoso commented Apr 24, 2020

Uh oh!

ktoso commented Apr 24, 2020

Uh oh!

yim-lee commented Apr 24, 2020

Uh oh!

ktoso commented Apr 27, 2020

Uh oh!

ktoso commented Apr 27, 2020

Uh oh!

ktoso commented Apr 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ktoso commented Apr 27, 2020

Uh oh!

ktoso commented Apr 27, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktoso commented Apr 28, 2020

Uh oh!

ktoso commented Apr 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ktoso commented Apr 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktoso Apr 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktoso May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktoso commented Apr 30, 2020

ktoso commented Apr 22, 2020 •

edited

Loading

ktoso commented Apr 24, 2020 •

edited

Loading

ktoso commented Apr 27, 2020 •

edited

Loading

ktoso commented Apr 28, 2020 •

edited

Loading

ktoso Apr 30, 2020 •

edited

Loading

ktoso May 1, 2020 •

edited

Loading