[Microservice] Port Manager 2.0 Implementation #301

chenpiaoping · 2020-07-17T08:06:21Z

This PR includes the following features:

Refactoring port-manager.
Vpc supports batch acquisition.
Subnet supports batch acquisition.
Mac address supports batch creation.
Ip address supports batch creation.
Security groups support batch acquisition and binding/unbinding.

xieus

Some early comments. I will continue to review the rest.

lib/src/main/java/com/futurewei/alcor/common/db/ignite/IgniteConfiguration.java

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

xieus · 2020-07-17T11:55:18Z

lib/src/main/java/com/futurewei/alcor/common/stats/DurationCalculator.java

+        if (startTimes.containsKey(id)) {
+            startTime = startTimes.get(id);
+        } else {
+            startTime = endTime;


Will this give us some false positive information that the duration is zero? :-)

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

...manager/src/main/java/com/futurewei/alcor/portmanager/service/implement/PortServiceImpl.java

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

xieus · 2020-07-17T12:00:17Z

..._manager/src/main/java/com/futurewei/alcor/privateipmanager/controller/IpAddrController.java

    @PostMapping("/ips")
    @ResponseBody
    @ResponseStatus(HttpStatus.CREATED)
+    @DurationStatistics


This is a very good annotation. Consider to apply it to all other services as well 👍

chenpiaoping · 2020-07-20T09:33:26Z

chenpiaoping · 2020-07-20T09:41:35Z

After optimization, the performance data for creating a single port is as follows:
Samples: 4877
Average: 10
Media: 10
90% Line: 12 (90% of the samples took no more than 12)
95% Line: 13 (93% of the samples took no more than 13)
99% Line: 17 (99% of the samples took no more than 17)
Min: 7
Maximum: 221
Error %: 0
Throughput: 81.3/sec
Received KB/sec: 95.60
Sent KB/sec: 42.31

Port configuration:
{
"port": {
"network_id": "5687abe4-9ae3-446a-979d-04d7e486646c",
"tenant_id": "3d53801c-32ce-4e97-9572-bb966f476ec",
"allowed_address_pairs": [{
"ip_address": "11.11.11.101",
"mac_address": "00:01:6C:06:A6:29"
}]
}
}

codecov-commenter · 2020-07-20T11:39:27Z

Codecov Report

Merging #301 into master will decrease coverage by 3.40%.
The diff coverage is 47.81%.

@@             Coverage Diff              @@
##             master     #301      +/-   ##
============================================
- Coverage     37.55%   34.15%   -3.41%     
- Complexity      550      859     +309     
============================================
  Files           241      353     +112     
  Lines          5680     8869    +3189     
  Branches        575     1078     +503     
============================================
+ Hits           2133     3029     +896     
- Misses         3292     5438    +2146     
- Partials        255      402     +147

Impacted Files	Coverage Δ	Complexity Δ
...i/alcor/apigateway/AlcorApiGatewayApplication.java	`33.33% <ø> (ø)`	`1.00 <0.00> (ø)`
...om/futurewei/alcor/dataplane/DataPlaneManager.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...a/com/futurewei/alcor/dataplane/config/Config.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...aplane/config/grpc/GoalStateProvisionerClient.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...ne/config/serialization/GoalStateDeserializer.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...lane/config/serialization/GoalStateSerializer.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...rewei/alcor/dataplane/controller/GSController.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...alcor/dataplane/exception/ACAFailureException.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...alcor/dataplane/exception/DPMFailureException.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...aplane/service/impl/MizarGoalStateServiceImpl.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
... and 318 more

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 8e72e78...5d034e9. Read the comment docs.

xieus · 2020-07-20T16:07:45Z

@chenpiaoping Some code conflict due to PR #304 that got just merged.

xieus · 2020-07-20T16:22:31Z

After optimization, the performance data for creating a single port is as follows:
Samples: 4877
Average: 10
Media: 10
90% Line: 12 (90% of the samples took no more than 12)
95% Line: 13 (93% of the samples took no more than 13)
99% Line: 17 (99% of the samples took no more than 17)
Min: 7
Maximum: 221
Error %: 0
Throughput: 81.3/sec
Received KB/sec: 95.60
Sent KB/sec: 42.31

Port configuration:
{
"port": {
"network_id": "5687abe4-9ae3-446a-979d-04d7e486646c",
"tenant_id": "3d53801c-32ce-4e97-9572-bb966f476ec",
"allowed_address_pairs": [{
"ip_address": "11.11.11.101",
"mac_address": "00:01:6C:06:A6:29"
}]
}
}

@chenpiaoping The result is outstanding. I will expedite the PR review and let us try to get the PR part of the release.

Couple of questions:

When we generate the test payload, do we automatically generate different configurations (uuid, ip address, mac address etc.) for different ports? This would impact performance as if we use the same configuration, we always hit the same record.
The size of samples is good to show some initial result. Can we increase to 10,000, 50,000, 100,000, and 500,000 to see if the latency and throughput curve of a single-node controller? Basically we want to find the breaking point.
Ignite: do we use one instance of DB, or multiple instances? Maybe that could be part of the metrics too.

Cheers!
@xieus

xieus

@chenpiaoping Some more comments, mostly minor.

I was wondering if we have a plan to recover IpAddrTest and IpRangeTest, which include good tests.

lib/src/main/java/com/futurewei/alcor/common/db/ignite/MockIgniteServer.java

xieus · 2020-07-21T00:12:26Z

.../port_manager/src/main/java/com/futurewei/alcor/portmanager/processor/AbstractProcessor.java

+*/
+package com.futurewei.alcor.portmanager.processor;
+
+import org.slf4j.Logger;


Can we use Alcor logger or you need something special from slf4j?

I thought alcor would use slf4j directly.

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

xieus · 2020-07-21T00:20:51Z

web/src/main/java/com/futurewei/alcor/web/entity/subnet/SubnetEntity.java

    }
+
+    public String getVpcId() {
+        return vpcId;


Does the @DaTa annotation not work here?

As i know, @ Data is no longer recommended because it generates code during compilation and has Poor security and maintainability

Okay thanks, let us remind the team of this annotation limit and start cleaning up the annotation when we see that.

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

xieus · 2020-07-21T00:26:18Z

.../src/main/java/com/futurewei/alcor/securitygroup/controller/SecurityGroupRuleController.java

 import static com.futurewei.alcor.securitygroup.utils.RestParameterValidator.*;

 @RestController
+@ComponentScan(value = "com.futurewei.alcor.common.stats")


@chenpiaoping Can you test the SG APIs from API GW to see if anything is expected? Recently there is a minor change of List SG API introduced by PR ##304. The List SG contract was not fully compatible wit Neutron. Can you check others as well.

Thanks.

You can do it a separate/small PR if any change is needed.

ok, i will do that.

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

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

chenpiaoping · 2020-07-22T08:22:12Z

When we generate the test payload, do we automatically generate different configurations (uuid, ip address, mac address etc.) for different ports? This would impact performance as if we use the same configuration, we always hit the same record.
> PM assigns random ip and mac address to ports that do not have ip and mac address
The size of samples is good to show some initial result. Can we increase to 10,000, 50,000, 100,000, and 500,000 to see if the latency and throughput curve of a single-node controller? Basically we want to find the breaking point.
>From the test result , we find that the QPS is about 81.3/sec. that is to say, it is about 5000 / min. If the rate of the sender exceeds this value, the average processing time of each request will increase.
Ignite: do we use one instance of DB, or multiple instances? Maybe that could be part of the metrics too.
> I deployed an ignite instance on my laptop. the only meaning of this test result is that it is much better than the previous test results in the same environment (my laptop).

chenpiaoping · 2020-07-25T07:53:35Z

@xieus
The problem of ignite deadlock, which I mentioned at the last regular meeting, appeared in the K8s environment:
"http-nio-8080-exec-6" #22 daemon prio=5 os_prio=0 cpu=70.00ms elapsed=635.62s tid=0x00007fddfe53d800 nid=0x1b runnable [0x00007fddb87a6000]
java.lang.Thread.State: RUNNABLE
at java.net.SocketInputStream.socketRead0(java.base@11.0.5/Native Method)
at java.net.SocketInputStream.socketRead(java.base@11.0.5/SocketInputStream.java:115)
at java.net.SocketInputStream.read(java.base@11.0.5/SocketInputStream.java:168)
at java.net.SocketInputStream.read(java.base@11.0.5/SocketInputStream.java:140)
at org.apache.ignite.internal.client.thin.TcpClientChannel$ByteCountingDataInput.read(TcpClientChannel.java:535)
at org.apache.ignite.internal.client.thin.TcpClientChannel$ByteCountingDataInput.readInt(TcpClientChannel.java:572)
at org.apache.ignite.internal.client.thin.TcpClientChannel.processNextResponse(TcpClientChannel.java:272)
at org.apache.ignite.internal.client.thin.TcpClientChannel.receive(TcpClientChannel.java:234)
at org.apache.ignite.internal.client.thin.TcpClientChannel.service(TcpClientChannel.java:171)
at org.apache.ignite.internal.client.thin.ReliableChannel.service(ReliableChannel.java:180)
at org.apache.ignite.internal.client.thin.GenericQueryPager.next(GenericQueryPager.java:71)
at org.apache.ignite.internal.client.thin.ClientQueryCursor$1.nextPage(ClientQueryCursor.java:93)
at org.apache.ignite.internal.client.thin.ClientQueryCursor$1.hasNext(ClientQueryCursor.java:76)
at org.apache.ignite.internal.client.thin.ClientQueryCursor.getAll(ClientQueryCursor.java:47)
at com.futurewei.alcor.common.db.ignite.IgniteClientDbCache.getAll(IgniteClientDbCache.java:182)
at com.futurewei.alcor.common.db.ignite.IgniteClientDbCache.getAll(IgniteClientDbCache.java:175)

chenpiaoping · 2020-07-27T02:47:00Z

@xieus This PR is ready to be reviewed.

chenpiaoping · 2020-07-27T03:55:07Z

The time cost of calling CreatePortBulk to create 50 port(Body contains 50 port) is as follows:

xieus

@xieus This PR is ready to be reviewed.

Thanks @chenpiaoping Somehow I missed this message. Let me do the review this afternoon.

xieus · 2020-07-25T05:06:53Z

...s/port_manager/src/main/java/com/futurewei/alcor/portmanager/processor/CallbackFunction.java

@@ -0,0 +1,9 @@
+package com.futurewei.alcor.portmanager.processor;
+
+


extra line.

xieus

We have Port Manager 2.0 now 👍

@chenpiaoping It may be a good idea for you to give some high-level introduction of new Port Manager design and implementation in the open-source meeting. This is a pretty decent implementation.

xieus · 2020-08-10T23:14:43Z

...ices/port_manager/src/main/java/com/futurewei/alcor/portmanager/processor/PortProcessor.java

+    }
+
+    @Override
+    void createProcess(PortContext context) {


This createProcess only applies to port with binding host id. How about those without binding host id?

xieus · 2020-08-10T23:22:26Z

...s/port_manager/src/main/java/com/futurewei/alcor/portmanager/processor/ProcessorManager.java

+import java.util.*;
+
+@Component
+public class ProcessorManager {


Can you give some comments to ProcessManager, AbstractProcess, PortProcess, and other types of processes etc.?

xieus · 2020-08-10T23:30:24Z

services/port_manager/src/main/java/com/futurewei/alcor/portmanager/request/RequestManager.java

+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
+
+public class RequestManager {


Please make some comments to RequestManager, and some other important requests as well.

chenpiaoping force-pushed the port-manager branch 2 times, most recently from 451e09a to d578881 Compare July 17, 2020 10:08

xieus reviewed Jul 17, 2020

View reviewed changes

xieus assigned chenpiaoping Jul 17, 2020

xieus added the refactor Code refactor and improvement label Jul 17, 2020

xieus added this to the Version 0.7.2020.07.30 milestone Jul 17, 2020

chenpiaoping force-pushed the port-manager branch 2 times, most recently from 13d5c8d to f752478 Compare July 20, 2020 01:55

chenpiaoping force-pushed the port-manager branch from 34acc08 to 439252a Compare July 20, 2020 10:49

xieus reviewed Jul 21, 2020

View reviewed changes

chenpiaoping force-pushed the port-manager branch from 439252a to 89faca6 Compare July 22, 2020 01:48

chenpiaoping force-pushed the port-manager branch 6 times, most recently from ece36ae to 3197a81 Compare July 24, 2020 04:44

chenpiaoping force-pushed the port-manager branch 2 times, most recently from c66982e to 8d593dc Compare July 27, 2020 02:13

xieus reviewed Jul 29, 2020

View reviewed changes

chenpiaoping force-pushed the port-manager branch from 8d593dc to f83279f Compare July 30, 2020 03:58

chenpiaoping force-pushed the port-manager branch from 0f917a2 to 9019891 Compare August 6, 2020 06:25

er1cthe0ne mentioned this pull request Aug 7, 2020

[Port Manager] need to send down the correct neighbor info to Data Plane Manager #343

Closed

chenpiaoping force-pushed the port-manager branch from 10515f2 to 5d034e9 Compare August 8, 2020 02:22

chenpiaoping added 22 commits August 10, 2020 14:59

Refactoring port manager

a577b17

Refactor: Delete port

69330bf

Refactor: Update port

10b7f1c

Refactor: Add some logs

56eaf25

Fix compile error

2ce05e8

private-ip-manager performance optimization

d4162ef

add thin ignite config item

e1415c9

Rest client supports batch interface

466166e

security group support query with parameters

335bc25

Close some useless logs

12f129c

Batch port binding security groups are supported

dfcebaa

Rebase from alcor master

e69dabe

Rebase from master

5da04e3

Allocate ip address by subnet id

b08026f

Rebase from master and fix some UTs error

62bc676

Build the correct network configuration when updating port

1e3a92d

Fix the problem of incorrect network configuration

ca4f753

Query node info by name

e8b70ba

Change the name of neighbor cache

9bc758d

Fix issue of Missing Neighbor Info in Cross-Node Scenario

85b410b

Fix the problem of missing tunnel id

6e23bb6

Fix the problem that take a long time to create an ip range

0827dc5

chenpiaoping force-pushed the port-manager branch from 5d034e9 to 0827dc5 Compare August 10, 2020 07:02

xieus approved these changes Aug 10, 2020

View reviewed changes

xieus changed the title ~~Refactoring Port Manager~~ [Microservice] Port Manager 2.0 Implementation Aug 10, 2020

xieus merged commit 76707cf into futurewei-cloud:master Aug 10, 2020

xieus linked an issue Oct 13, 2020 that may be closed by this pull request

[Integration] Missing Neighbor Info in Cross-Node Scenario #335

Closed

		@@ -0,0 +1,9 @@
		package com.futurewei.alcor.portmanager.processor;

[Microservice] Port Manager 2.0 Implementation #301

[Microservice] Port Manager 2.0 Implementation #301

Uh oh!

Conversation

chenpiaoping commented Jul 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xieus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chenpiaoping commented Jul 20, 2020

Uh oh!

chenpiaoping commented Jul 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jul 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

xieus commented Jul 20, 2020

Uh oh!

xieus commented Jul 20, 2020

Uh oh!

xieus left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chenpiaoping commented Jul 22, 2020

Uh oh!

chenpiaoping commented Jul 25, 2020

Uh oh!

chenpiaoping commented Jul 27, 2020

Uh oh!

chenpiaoping commented Jul 27, 2020

Uh oh!

xieus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

chenpiaoping commented Jul 17, 2020 •

edited

Loading

chenpiaoping commented Jul 20, 2020 •

edited

Loading

codecov-commenter commented Jul 20, 2020 •

edited

Loading