-
Notifications
You must be signed in to change notification settings - Fork 33
[DPM] Add gRPC/MQ auto switch mechanism #742
Conversation
case "MIXED": | ||
return choosePath(portEntity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VanderChen The "Mixed" mode for me seems not a real MIXED mode, doesn't it? I think "AUTO" is better to describe this mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cj-chung Good suggestion. The mode name is changed to "AUTO".
if (numberOfPorts > UPPER_VPC_SIZE) { | ||
chosenPath = !USE_GRPC; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VanderChen Why the line#105 shows that using USE_GRPC
when numberOfPorts > UPPER_VPC_SIZE
, but the line#120 shows that using !USE_GRPC
in the same condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AtomicInteger atomicNumberOfPorts = new AtomicInteger(); | ||
List<String> subnetIds; | ||
subnetIds = vpcSubnetsCache.getVpcSubnets(vpcId).getSubnetIds(); | ||
subnetIds.stream().forEach(subnetId -> { | ||
try { | ||
InternalSubnetPorts internalSubnetPorts = subnetPortsCache.getSubnetPorts(subnetId); | ||
atomicNumberOfPorts.addAndGet(internalSubnetPorts.getPorts().size()); | ||
} catch (Exception e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VanderChen Do we have a better way to retrieve the VPC size (total number of ports) instead of calculating the size on the fly. I am worry about the latency here if it's a very big VPC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cj-chung Another option is counting in VpcPathCache. When creating ports, the counter in the cache is incremented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VanderChen Please create an issue for this problem and link to this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (numberOfPorts > UPPER_VPC_SIZE) { | ||
vpcPathCache.setPath(vpcId, USE_GRPC); | ||
return !USE_GRPC; | ||
} else { | ||
vpcPathCache.setPath(vpcId, !USE_GRPC); | ||
return USE_GRPC; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VanderChen line#107 (USE_GRPC
) and line #108 is not match (return !USE_GRPC
). Same as line#110 and line#111.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cj-chung I have fixed this and use USE_MQ to replace ! USE_GRPC for clearer expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
In this PR,
The gRPC only mode has passed the Jenkins test.