feat: Allow indirect replication #1070

patriknw · 2023-11-15T14:03:19Z

Draft because requires a change in Akka first akka/akka#32233

patriknw · 2023-11-15T14:03:45Z

...tests/src/it/scala/akka/projection/grpc/replication/IndirectReplicationIntegrationSpec.scala

+import org.scalatest.wordspec.AnyWordSpecLike
+import org.slf4j.LoggerFactory
+
+object IndirectReplicationIntegrationSpec {


This is similar to existing ReplicationIntegrationSpec

patriknw · 2023-11-15T14:05:44Z

...ion-grpc-tests/src/test/scala/akka/projection/grpc/replication/ReplicationSettingsSpec.scala

+           # only events from the origin replica will be transferred from the origin
+           # replica. When each replica is connected to each other replica it's most
+           # efficient to disable indirect replication.
+           indirect-replication = on


default value is off, this is included in docs and maybe we should show off
however, then the test would be testing this

Make this "off" and enable it after snippet end or through config factory apis?

patriknw · 2023-11-15T14:07:27Z

...ojection-grpc/src/main/scala/akka/projection/grpc/replication/internal/ReplicationImpl.scala

+          if (settings.indirectReplication || meta.originReplica == settings.selfReplicaId)
+            Future.successful(envelope.eventOption)
+          else
+            filteredEvent // Optimization: was replicated to this replica, don't pass the payload across the wire


This is the reason why I haven't enabled it by default. Would have a substantial performance overhead. We'll see where it is going when we support star topologies and such.

It would actually be nice to configure this per replica, but here on the producer side we don't know which replica is the consumer (at least I couldn't see that)

If it was a predicate instead of a setting (or a pattern) the user could design for a star-topology-replica naming strategy and only replicate indirectly only to the edge nodes.

yes, need to think this through again. we want to support a full mesh in cloud where indirect-replication=off, and edge connecting in to one or a few cloud replicas. edge consumers need indirect-replication=on

Maybe we could look at the allReplicas (or otherReplicas) and use that for deciding indirect. If it's included in the defined allReplicas then off, otherwise on. However, again, here on the producer side we don't know the replicaId of the consumer. Maybe we have to add that?

We want to indirectly replicate from cloud to edge, and from edge to other cloud replicas, so maybe it is more about knowing which replication goes/comes from where when deciding (I haven't completely thought this all the way through)?

indirectly ... from edge to other cloud replicas

true, didn't think about that one

It is the consumer side that knows what other replicas it is consuming from, otherReplicas. Therefore it would be wrong to configure or decide the indirect aspect on the producer side, but we want the actual filtering of indirect events to happen on the producer side.

We could include the otherReplicas in the InitReq from the consumer. On the producer it would then use that to decide if indirect events should be included or not. If event origin is in initReq.otherReplicas then it can be filter out, otherwise included (as indirect event).

patriknw

New approach, would like feedback before continuing...

patriknw · 2023-11-20T16:30:27Z

akka-projection-grpc/src/main/protobuf/akka/projection/grpc/event_producer.proto

+  // Used for Replicated Event Sourcing, the replica id of the consumer
+  string replica_id = 6;
+  // Used for Replicated Event Sourcing, other replicas that the consumer is connected to.
+  repeated string other_replica_ids = 7;


@johanandren I have tried this instead. These replica ids are included from the consumer in the InitReq. Then that is used on the producer side to decide if an event based on its origin should be emitted or handled as FilteredEvent.

For producer push it would be similar. These fields would be included in the ConsumerEventStart response from the consumer side and then used on the producer side.

Ok, seems reasonable.

patriknw · 2023-11-20T16:34:36Z

...ection-grpc/src/main/scala/akka/projection/grpc/replication/internal/EventOriginFilter.scala

+        case Some(meta: ReplicatedEventMetadata) =>
+          !exclude(meta.originReplica)
+        case _ =>
+          // FIXME eventMetadata isn't loaded by backtracking


It's working aside from this little detail. For ordinary transformation, which we used for the origin filtering, we solve it by not applying transformation for backtracking and instead do that in loadEvent. It would be difficult to do the same for this, because then we would need the replica ids also in the load event request.

Do you think it's fine to change the r2dbc query to always load metadata, also for backtracking?

Sleeping on this. Feels fragile to not use the same origin filter from loadEvent also. Even though we only use loadEvent from backtracking events I think it should have the same transformation and filtering mechanism. I think it's best to include those additional replica id fields also in the loadEvent request.

It would be difficult to do the same for this

no, that shouldn't be difficult

I agree, let's go with always passing metadata and applying the same transform/filter.

patriknw · 2023-11-21T13:39:35Z

Incorporated the filter changes from #1072 because there were too many changes in same place. Also added test for such filters.

johanandren · 2023-11-29T09:52:47Z

Rebased and bumped Akka to get this ready for merge

* and define filters via ReplicationSettings * deprecate grpcReplication with producer filter params, via settings instead

patriknw · 2023-11-29T12:17:04Z

Don't know what happened, but this didn't contain my latest work. Restored from history so now this should be ready.

johanandren · 2023-11-29T12:20:48Z

Maybe I missed pulling the latest before rebasing, sorry!

patriknw · 2023-11-29T12:27:21Z

akka-projection-grpc/src/main/protobuf/akka/projection/grpc/event_consumer.proto

@@ -59,6 +59,7 @@ message ConsumerEventInit {

 message ConsumerEventStart {
  repeated FilterCriteria filter = 1;
+  ReplicaInfo replica_info = 6;


added also to ConsumerEventStart but setting it and implementing corresponding filter on the producer side for RES must be done in @johanandren 's #1073 (or after that)

I'll do that in that PR once rebased on top of this after merged

johanandren

LGTM

patriknw · 2023-11-29T13:40:47Z

Test fail was in IntegrationSpec, which I think is unrelated. #905
However, I see it failing consistently locally, but not in main.

patriknw · 2023-11-29T13:48:09Z

ah, that was because I changed log level. Some tests needed trace level. Should work now. 🤞

patriknw commented Nov 15, 2023

View reviewed changes

patriknw force-pushed the wip-res-indirect-patriknw branch from aa62b55 to a9a4f1c Compare November 17, 2023 13:27

patriknw commented Nov 20, 2023

View reviewed changes

patriknw mentioned this pull request Nov 20, 2023

Feat: edge initiated replication #1073

Merged

patriknw marked this pull request as ready for review November 21, 2023 13:37

patriknw mentioned this pull request Nov 21, 2023

feat: RES initial consumer filter #1072

Closed

1 task

johanandren force-pushed the wip-res-indirect-patriknw branch from 55b02cc to d4cf2a8 Compare November 29, 2023 09:52

patriknw force-pushed the wip-res-indirect-patriknw branch from d4cf2a8 to 41147c8 Compare November 29, 2023 12:02

patriknw added 6 commits November 29, 2023 13:14

feat: Allow indirect replication

803686a

javadsl

bf40695

new take, replica ids included in InitReq

854ea2e

filter origin also from loadEvent

5b512d7

feat: RES initial consumer filter

9d26050

* and define filters via ReplicationSettings * deprecate grpcReplication with producer filter params, via settings instead

test of edge consumer filter

c6f523c

patriknw force-pushed the wip-res-indirect-patriknw branch from 41147c8 to c6f523c Compare November 29, 2023 12:15

mima filter

72c7578

add to ConsumerEventStart

e1abef2

patriknw commented Nov 29, 2023

View reviewed changes

johanandren approved these changes Nov 29, 2023

View reviewed changes

patriknw added 2 commits November 29, 2023 13:28

headers

8d29177

fix test race condition

c8b0f0f

trace level needed for test

ff96d7c

johanandren merged commit 3670804 into main Nov 29, 2023
20 checks passed

johanandren deleted the wip-res-indirect-patriknw branch November 29, 2023 13:58

johanandren added this to the 1.5.1 milestone Nov 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Allow indirect replication #1070

feat: Allow indirect replication #1070

patriknw commented Nov 15, 2023

patriknw Nov 15, 2023

patriknw Nov 15, 2023

johanandren Nov 15, 2023

patriknw Nov 17, 2023

patriknw Nov 15, 2023

patriknw Nov 15, 2023

johanandren Nov 15, 2023

patriknw Nov 17, 2023

johanandren Nov 17, 2023

patriknw Nov 17, 2023

patriknw Nov 17, 2023

patriknw left a comment

patriknw Nov 20, 2023

patriknw Nov 21, 2023

johanandren Nov 21, 2023

patriknw Nov 20, 2023

patriknw Nov 21, 2023

johanandren Nov 21, 2023

patriknw commented Nov 21, 2023

johanandren commented Nov 29, 2023

patriknw commented Nov 29, 2023

johanandren commented Nov 29, 2023

patriknw Nov 29, 2023

johanandren Nov 29, 2023

johanandren left a comment

patriknw commented Nov 29, 2023

patriknw commented Nov 29, 2023

feat: Allow indirect replication #1070

feat: Allow indirect replication #1070

Conversation

patriknw commented Nov 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patriknw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patriknw commented Nov 21, 2023

johanandren commented Nov 29, 2023

patriknw commented Nov 29, 2023

johanandren commented Nov 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanandren left a comment

Choose a reason for hiding this comment

patriknw commented Nov 29, 2023

patriknw commented Nov 29, 2023