[Data-Plane Mgr] Support GoalStateV2 for UnicastGS and MulticastGS #625

VanderChen · 2021-06-04T05:45:01Z

No description provided.

codecov-commenter · 2021-06-04T05:57:28Z

Codecov Report

Merging #625 (18aaa21) into master (620df1a) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #625   +/-   ##
=========================================
  Coverage     31.90%   31.90%           
  Complexity     1249     1249           
=========================================
  Files           525      525           
  Lines         13490    13490           
  Branches       1669     1669           
=========================================
  Hits           4304     4304           
  Misses         8598     8598           
  Partials        588      588

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 620df1a...18aaa21. Read the comment docs.

xieus · 2021-06-05T05:26:12Z

...ata_plane_manager/src/main/java/com/futurewei/alcor/dataplane/entity/UnicastGoalStateV2.java

+
+    public UnicastGoalStateByte getUnicastGoalStateByte() {
+        UnicastGoalStateByte unicastGoalStateByte = new UnicastGoalStateByte();
+        unicastGoalStateByte.setNextTopic(this.topic);


@VanderChen We would need to setNextSubTopic as well, right?

This involves another question, whether use Pulsar worker to forward messages. This design was originally used to solve the multiple sending problem of multicastGS. DPM only needs to send GS once, and the worker will complete it multiple times. To be consistent, UnicastGS also uses the worker forwarding method.
i.e.
DPM(UniWorkerTopic, (NextTopic, GS)) -- (NextTopic, GS)--> Pulsar Worker(NextTopic, GS) --(GS)--> ACA
DPM(MultiWorkerTopic, (List, GS)) --(List, GS)--> Pulsar Worker(Traversal_NextTopic, GS) --(GS)--> ACA
Thus, considering compatibility with the current version and not sending more information to the worker, I did not add it. I'm not sure if this is the right thing to do.
There are two options for those containing SubTopic. One is not to use workers, this does not require the UnicastGoalStateByte class. The other is to deploy new Pulsar Workers. I prefer the former one.
i.e.
DPM((Topic, SubTopic), GS) --(GS)--> ACA

I am not quite sure what's problem here. Where is the Pulsar worker located? In the DPM (such as pulsarClient in the pulsar\DataPlaneClientImpl.java) or somewhere else?
Does your prefer way to eliminate Pulsar worker and rely on the GRPC's unicast and multicast goalstates?

Do you have this problem resolved? Let us discuss it in today's open-source meeting.

...a_plane_manager/src/main/java/com/futurewei/alcor/dataplane/entity/MulticastGoalStateV2.java

xieus · 2021-07-03T06:21:20Z

We need to move this PR forward @VanderChen

…o mq-dpmv2

xieus

@VanderChen A few more comments.

xieus · 2021-08-11T21:17:26Z

...s/data_plane_manager/src/main/java/com/futurewei/alcor/dataplane/client/DataPlaneClient.java

    List<String> sendGoalStates(List<UnicastGoalState> unicastGoalStates,
                                MulticastGoalState multicastGoalState) throws Exception;
+
+//    List<String> sendGoalStates(List<UnicastGoalStateV2> unicastGoalStates) throws Exception;


Remove the commented line if not needed.

xieus · 2021-08-11T21:26:22Z

...ata_plane_manager/src/main/java/com/futurewei/alcor/dataplane/entity/UnicastGoalStateV2.java

+
+    public UnicastGoalStateByte getUnicastGoalStateByte() {
+        UnicastGoalStateByte unicastGoalStateByte = new UnicastGoalStateByte();
+        unicastGoalStateByte.setNextTopic(this.topic);


Do you have this problem resolved? Let us discuss it in today's open-source meeting.

...a_plane_manager/src/main/java/com/futurewei/alcor/dataplane/entity/MulticastGoalStateV2.java

xieus · 2021-08-11T21:39:22Z

web/src/main/java/com/futurewei/alcor/web/entity/dataplane/MulticastGoalStateByte.java


 public class MulticastGoalStateByte {
    private List<String> nextTopics;
+    private List<String> nextSubTopics;


Does adding this new field break any existing workflow?

xieus · 2021-08-11T21:41:20Z

There are two conflicting files:
services/data_plane_manager/src/main/java/com/futurewei/alcor/dataplane/service/impl/NeighborService.java
services/data_plane_manager/src/main/java/com/futurewei/alcor/dataplane/service/impl/RouterService.java

…o mq-dpmv2

lgtm-com · 2021-09-14T13:03:25Z

This pull request introduces 7 alerts when merging 3ca09c0 into 2cc0269 - view on LGTM.com

new alerts:

7 for Useless null check

…q-dpmv2

lgtm-com · 2021-09-22T13:13:40Z

This pull request introduces 8 alerts when merging 9b6bc7f into 016b3e9 - view on LGTM.com

new alerts:

7 for Useless null check
1 for Container contents are never accessed

…q-dpmv2

lgtm-com · 2021-09-29T17:51:44Z

This pull request introduces 8 alerts when merging 14a4738 into 016b3e9 - view on LGTM.com

new alerts:

7 for Useless null check
1 for Container contents are never accessed

…o mq-dpmv2

lgtm-com · 2021-10-16T14:25:34Z

This pull request introduces 8 alerts when merging aa76721 into 91af57e - view on LGTM.com

new alerts:

7 for Useless null check
1 for Container contents are never accessed

VanderChen · 2021-10-17T07:54:52Z

All tests in Jenkins have passed

cj-chung

LGTM.

Those overloaded functions for goalstateV2 under dataplane/service/impl/*Service.java may not needed anymore after we have new version of DPM for goalstateV2 and NCM, but for now we can merge it first for goalstateV1 and goalstateV2 compatibility.

xieus · 2021-10-20T21:31:40Z

services/data_plane_manager/src/main/resources/application.properties

 mq.type=pulsar
 #####Pulsar configuration#####
-pulsar.url=pulsar://localhost:6650
+pulsar.url=pulsar://192.168.1.105:6650


@VanderChen It is the best practice not checking in any configuration specific to a dev environment. Please consider changing it in your next PR #695.

Add UnicastGS and MultiGS based on GoalStateV2

021d12b

xieus requested review from cj-chung and xieus June 5, 2021 05:12

xieus assigned VanderChen Jun 5, 2021

xieus added this to the Version 0.17.2021.07.30 milestone Jun 5, 2021

xieus suggested changes Jun 5, 2021

View reviewed changes

VanderChen added 7 commits June 5, 2021 21:13

Add VpcUnicastGoalStateByte and VpcMulticastGoalStateByte

18aaa21

Implment DpmService V2

2970780

Implement PortService.buildPortStateV2

b6ab7c4

Implement VpcService.buildVpcStates V2

6b21941

Implement VpcService.buildVpcStates V2

09f118c

Build UnicastGoalStateV2 and MulticastGoalStateV2

e15f232

Build UnicastGoalStateV2 and MulticastGoalStateV2

2f64247

VanderChen added 5 commits July 8, 2021 15:40

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

5d1e2e5

…o mq-dpmv2

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

73f44cc

…o mq-dpmv2

Fix patchGoalstateForNeighbor

0fa81ed

Fix patchGoalstateForNeighbor

b4ccd2f

Fix DpmService

4ff2ff8

xieus changed the title ~~Add UnicastGS and MultiGS based on GoalStateV2~~ Add UnicastGS and MulticastGS based on GoalStateV2 Aug 11, 2021

xieus added enhancement New feature or request feature feature development labels Aug 11, 2021

xieus reviewed Aug 11, 2021

View reviewed changes

VanderChen added 4 commits August 15, 2021 21:49

Solve conflicts

a8e0eea

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

59913ae

…o mq-dpmv2

Support GSv2 in DataPlaneClint

39673c3

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

3ca09c0

…o mq-dpmv2

VanderChen added 5 commits September 15, 2021 01:01

Merge branch 'mq-dpmv2' of https://github.com/VanderChen/alcor into m…

e5e6083

…q-dpmv2

Implement DataPlaneClientV2 for gRPC

cceb87c

Merge branch 'mq-dpmv2' of https://github.com/VanderChen/alcor into m…

988d65a

…q-dpmv2

Implement DataPlaneClientV2 for gRPC

6bfe9e2

Implement DataPlaneClientV2 for gRPC

9b6bc7f

VanderChen added 2 commits September 30, 2021 00:28

Merge branch 'mq-dpmv2' of https://github.com/VanderChen/alcor into m…

a575a98

…q-dpmv2

Fix service bean bug

14a4738

VanderChen added 2 commits October 16, 2021 12:34

Merge branch 'master' of https://github.com/futurewei-cloud/alcor int…

d157af9

…o mq-dpmv2

Fix bug

aa76721

VanderChen requested a review from xieus October 17, 2021 07:53

VanderChen mentioned this pull request Oct 19, 2021

Enable pulsar client support VPC-topic mode based on GSv1 and GSv2 #695

Merged

cj-chung requested a review from DavidLiu506 October 19, 2021 17:24

cj-chung approved these changes Oct 20, 2021

View reviewed changes

xieus reviewed Oct 20, 2021

View reviewed changes

xieus changed the title ~~Add UnicastGS and MulticastGS based on GoalStateV2~~ [Data-Plane Mgr] Support GoalStateV2 for UnicastGS and MulticastGS Oct 20, 2021

xieus self-requested a review October 20, 2021 21:35

xieus approved these changes Oct 20, 2021

View reviewed changes

xieus merged commit 50c6563 into futurewei-cloud:master Oct 20, 2021

[Data-Plane Mgr] Support GoalStateV2 for UnicastGS and MulticastGS #625

[Data-Plane Mgr] Support GoalStateV2 for UnicastGS and MulticastGS #625

Uh oh!

Conversation

VanderChen commented Jun 4, 2021

Uh oh!

codecov-commenter commented Jun 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

Uh oh!

xieus commented Jul 3, 2021

Uh oh!

xieus left a comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xieus commented Aug 11, 2021

Uh oh!

lgtm-com bot commented Sep 14, 2021

Uh oh!

lgtm-com bot commented Sep 22, 2021

Uh oh!

lgtm-com bot commented Sep 29, 2021

Uh oh!

lgtm-com bot commented Oct 16, 2021

Uh oh!

VanderChen commented Oct 17, 2021

Uh oh!

cj-chung left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Jun 4, 2021 •

edited

Loading