Register consecutive priorities in batch #1331
Conversation
|
Thanks for your PR. The following commands are available:
|
Codecov Report
@@ Coverage Diff @@
## master #1331 +/- ##
==========================================
- Coverage 64.60% 64.49% -0.11%
==========================================
Files 157 159 +2
Lines 12626 12667 +41
==========================================
+ Hits 8157 8170 +13
- Misses 3622 3644 +22
- Partials 847 853 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Dyanngg
force-pushed
the
batch-priority
branch
2 times, most recently
from
56bd497
to
c832a83
Compare
I have updated UT and I believe it now covers all the cases mentioned in the PR description. Also, I realized that the original PR description for case 4 is inaccurate, I have updated it accordingly as well. |
|
/test-all |
| for i := target; i >= 0; i-- { | ||
| if cost, exists := costMap[i]; exists && cost.cost < minCost { | ||
| // make sure that the reassign range makes up for all Priorities to be registered. | ||
| if int(cost.upperBound-cost.lowerBound)+1 == numNewPriorities+cost.cost { |
There was a problem hiding this comment.
isn't this guaranteed by your algorithm above? If yes, wouldn't it make sense to log something or return an error if this doesn't hold?
There was a problem hiding this comment.
No it is not guaranteed by the algorithm above and there should not be an error thrown. Consider a case where we need to find 3 empty spots below lowerBound and/or above upperBound. In the first pass, the algorithm tries to fill the table on the cost and where the lowerBound would be, if we were to use 1, 2 and 3 lower empty spots. For 1 and 2, when a cost is found, the upperBound will be temporarily set to the current upperBound which has not shifted at all. Those costs however only makes sense if in the second pass, the algorithm is able to find cost of getting 2 and 1 empty spots from above, in which case the total cost and new upperBound will be updated.
In other words, when int(cost.upperBound-cost.lowerBound)+1 != numNewPriorities+cost.cost, it simply means that this combination of (use x empty slots below, use y empty slots above) is not feasible based on the current map, but it's not an error.
Dyanngg
force-pushed
the
batch-priority
branch
from
a05bc2c
to
a90b986
Compare
|
/test-all |
Codecov Report
@@ Coverage Diff @@
## master #1331 +/- ##
=======================================
Coverage 64.32% 64.32%
=======================================
Files 159 159
Lines 12674 12684 +10
=======================================
+ Hits 8152 8159 +7
+ Misses 3668 3667 -1
- Partials 854 858 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Dyanngg
force-pushed
the
batch-priority
branch
from
a90b986
to
6a1f0bd
Compare
|
/test-all |
This PR aims to improve the efficiency of registering Antrea-native policy priorities in the OpenFlow priority space. Fundamental idea is to take advantage of the fact that, when the agent receives internal NP created for those policies, it already knows the number of absolute consecutive Priorities that needs to be registered. Those Priorities will have the same Tier and Policy priority, and consecutive rule priorities as they correspond to the 1st, 2nd, and etc. rules of the ingress/egress rule of that policy.
Thus, when registering priorities, we can break these priorities into zones (each zone of same Tier and Policy priority), and register each zone in batch. This eliminates the problematic scenario where asynchronously registering a series of priorities cause existing priorities to be reassigned each time.
The priorityAssigner module is refactored to accommodate this change. The following illustrations demonstrates the new algorithm that it uses to insert consecutive priorities. For each diagram, the first row depicts the current ofPriority space, and where the initial heuristic function puts in the inserting priorities.
Case 1 The spaces computed by the heuristic function is unoccupied, AND the priorities immediately lower than those being inserted, if exists, are registered below these ofPriorities, AND the priorities immediately higher than those being inserted, if exists, are registered above these ofPriorities. In this case, we simply assign the ofPriorities returned by the heuristic function.
Case 2 The ofPriorities returned by the heuristic function overlaps with existing priorities/are out of place. If the priorities to be registered overlaps with lower priorities/are lower than the lower priorities, and the gap between lowerBound for insertion and upperBound for insertion is very large, then we insert these priorities above the lowerBound, offsetted by a constant zoneOffset. This offset gives buffer in case priorities are again created in between those zone. Vice versa for priorities overlaps with/higher than higher priorities.
Case 3 The ofPriorities returned by the heuristic function overlaps with existing priorities/are out of place, and the gap between lowerBound for insertion and upperBound for insertion is tight. In that case, we simply register the priorities in the middle of the gap.
Case 4 The gap between lowerBound for insertion and upperBound for insertion is not big enough for insertion. In this case, we push lower priorities and/or higher priorities towards each side in attempt to make enough room for the registering priorities, while in the meantime try to reassign as few existing priorities as possible. For example, in the following diagram, the yellow Priorities will be kept where they were to save reassignments.
