Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SDFAB-1100] Leverage in-order FlowRuleService APIs #512

Merged
merged 3 commits into from
Mar 28, 2022
Merged

Conversation

pierventre
Copy link
Collaborator

@pierventre pierventre commented Mar 23, 2022

FabricUpfProgrammable leverages the new APIs to guarantee in-order processing of the requests coming from the north.

If batch APIs are not used there is no guarantee about the processing order in the FlowRuleService. Instead, the striped API allow the apps to signal a preference in the requests processing which is used by the FlowRuleService to serialize
all the requests, having equal key, on the same executor.

Depends on https://gerrit.onosproject.org/c/onos/+/25423

FabricUpfProgrammable leverages the new APIs to guarantee
in-order processing of the requests coming from the north.

If batch APIs are not used there is no guarantee about the
processing order in the FlowRuleService. Instead, the `striped`
API allow the apps to signal a preference in the requests
processing which is used by the FlowRuleService to serialize
all the requests, having equal key, on the same executor.

Depends on https://gerrit.onosproject.org/c/onos/+/25423
Copy link
Member

@ccascone ccascone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I guess you want to wait to refactor key handling in ONOS before merging this.

@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #512 (08d9e80) into main (2e63f6d) will decrease coverage by 0.03%.
The diff coverage is 71.42%.

@@             Coverage Diff              @@
##               main     #512      +/-   ##
============================================
- Coverage     69.96%   69.92%   -0.04%     
+ Complexity      740      739       -1     
============================================
  Files            63       63              
  Lines          4801     4802       +1     
  Branches        530      530              
============================================
- Hits           3359     3358       -1     
- Misses         1164     1165       +1     
- Partials        278      279       +1     
Impacted Files Coverage Δ
...abric/tna/behaviour/upf/FabricUpfProgrammable.java 62.14% <71.42%> (-0.12%) ⬇️
...atumproject/fabric/tna/stats/StatisticDataKey.java 96.55% <0.00%> (-3.45%) ⬇️

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 2e63f6d...08d9e80. Read the comment docs.

@ccascone
Copy link
Member

Only codecov failed. Merging anyways since the diff with the main branch is negligible (-0.04%).

@ccascone ccascone merged commit 126aba9 into main Mar 28, 2022
@ccascone ccascone deleted the inorder-flow branch March 28, 2022 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants