Tackling epic_mef_eline_flows_error_handling

[Alopalao]

The scope of this epic is to handle errors detected by installing and deleting flows from EVC paths. Once this epic is finished the issue #476 will be closed.

Cause of incorrect installation and deletion of flows

TLDR: Requests for flows are sent by the number of switches used in an EVC. This number rises fast when dealing with 100s EVCs in a short time causing Service Unavailable.

Why this error can happen

When managing hundreds of EVCs, it is possible to get the error Service Unavailable from flow_manager. The API request port from flow_manager is the one that breaks first because of the amount of requests sent from EVCs. For example:
For an EVC from 00:01 (200) to 00:02 (200) non-NNI:

• current_path: Flows:

    00:01:1   --> Flow using VLAN 200
    00:01:3   --> Flow using VLAN 1
    00:02:2   --> Flow using VLAN 1
    00:02:1   --> Flow using VLAN 200

• failover_path: (ignoring UNIs aka. skip) Flows:

    00:01:4   --> Flow using VLAN 1
    00:03:3   --> Flow using VLAN 1
    00:03:2   --> FLow using VLAN 1
    00:02:3   --> Flow using VLAN 1

flow_manager allows installation by switch (or all switches), mef_eline sends a request per switch.
From the previous example, the number of flows per path would be:

• current_path -> 2 requests to install 4 flows (2 00:01 and 2 00:02)
• failover_path -> 3 requests to install 4 flows (1 00:01, 2 00:03 and 1 00:02)

In total, to create this EVC, 5 requests are sent. This causes the error Service Unavailable when the number of EVCs is high in the 100s.

Problem with the current handling of this error

TLDR: The current implementation tries to delete paths that errored by sending more requests which are very likely to get Service Unavailable. Path is deleted in mef_eline but flows are still installed.

More details in current approach

If an error Service Unavailable occurs, the next request will likely have the same error. If the installation of the EVC fails in the second switch for the current_path, for example:

    00:01:1   --> Flow using VLAN 200   -   INSTALLED
    00:01:3   --> Flow using VLAN 1     -   INSTALLED

    00:02:2   --> Flow using VLAN 1     -   ERRORED
    00:02:1   --> Flow using VLAN 200   -   ERRORED

mef_eline detects the error and tries to send more requests to delete everything related to EVC. But more EVCs does not help with Service Unavailable. So the EVC errors in all flows

    00:01:1   --> Flow using VLAN 200   -   NOT DELETED
    00:01:3   --> Flow using VLAN 1     -   NOT DELETED

    00:02:2   --> Flow using VLAN 1     -   NOT DELETED
    00:02:1   --> Flow using VLAN 200   -   NOT DELETED

This left flows installed. The path was deleted which was the only reference of these flows existing.

Solution for errored paths and approach to minimize the occurrence of Service Unavailable

Keep the failed path, add `status_reason` and try again later, related issue #511

The error Service Unavailable cannot be avoided. With enough number of requests, it will eventually happen.
This area is to deal with this and any other error while installing/deleting flows.

Sending more requests when a path fails is not ideal. Instead, we could implement a new field for the errored EVC.
status_reason (previously called error_status) field will be set as:

{"status_reason": {
    "current_path": "{action}, {error}",
    "failover_path": "{action}, {error}"
    }
}

Where action specifies what needs to be done so the EVC is fixed, it can be Redeploy|Delete. Error is the message from the exception caught.

New basic mechanic for path handling

In the case of path installation:

1. Deactivate EVC if it is installing current_path
2. Delete the existing path only if it exists. Avoid to send requests when there is no path.
   2.1. Path deletion errored, log error, set error in status_error, leave previous path unchanged, sync
   2.2. Return
3. Clean up status_reason
4. Create flows and send requests to install the path
   6.1. Path deletion errored, log error, set error in status_error, set new path, sync
   6.2. Return
5. Set new path
6. Activate EVC
7. Return

In the case of path deletion:

1. Get the previous path to be deleted if current_path or failover_path is being deleted
   1.1.1. Path does not exist
   1.1.2. Return
2. Create flows and send requests
   2.1. Requests errored, log error
   2.2. Raise exception EVCPathNotDeleted (New exception)
3. Return

Additional changes

• Add exception EVCPathNotDeleted to be raised when a deletion error happens.
• Handle exception EVCPathNotDeleted when deleting EVC. If an error happens, the EVC will not be deleted. It will just be disabled and deactivated.
• Add UI element to identify EVC with status_error, issue UI: Show that an EVC has an error #510
• Add request to deal with EVCs with status_error, issue Error handling when installing and deleting flows #495. More information in the next label

API requests dealing with EVC with failed paths, issue #495

Consistency check can handle EVC with status_error but the user can do it as well. (Changes are due)

Add a POST request to schedule a consistency check for EVCs with detected errors so these are corrected. Depending on how consistency check version 2 evolves this issue will differ a bit in its implementation.

Additional changes

• To avoid search for EVC, add a new variable self.failed_circuits to keep track of any failed EVCs.
• Add a new key filter to GET request for getting circuits. The URL would look like .../api/kytos/mef_eline/v2/evc/?failed_circuits=true.

Reduce the number of requests, related issue #514 (Implementing)

Service Unavailable is tied to the number of requests. So sending a single request per path reduces significantly the possibility of this error showing again.

A new request in flow_manager -> POST .../api/kytos/flow_manager/v2/flows/flows_by_switch with content:

{
    "00:00:00:00:00:00:00:03": {
        "flows": [{
                "cookie": 3,
                "priority": 500,
                "match": {"in_port": 1},
        }],
        "force": true
    },
    "00:00:00:00:00:00:00:02": {
        "flows": [{
                "cookie": 3,
                "priority": 200,
                "instructions": [{
                    "instruction_type": "apply_actions",
                    "actions": [{"action_type": "output", "port": 3}]
                }]
        }],
        "force": true
    }
}

Extra issues:

• From flow_manager, issue. Implement the receiving of flows in a bulk of switches.

Note: Scope from epic_mef_eline_consistency_v2 is higher and can close this epic. Keeping an eye how that epic evolves.

[Alopalao]

The only remaining entry points are when mef_eline creation or deletion gets blasted, is that correct?

Yes, I am realizing now that these planned changes deal more when an error is caught and change the previous behavior (try more requests). Essentially, this frees the traffic for other elements to continue. Also, this is true for any changes with what API can handle.
An easy way to test is to perform a link flap with a big enough number of EVCs to cause a 'Service Unavailable' message.

[viniarck]

Right. Got it.

[Alopalao]

Sure, I will change the name of the field to "status_reason".

[viniarck]

Sounds good. Let's go for it then.

[Alopalao]

Oh ye, I forgot about deactivation. Adding it.

[viniarck]

Cool. Aldo, what about the failover_path if it fails the removal or installation? In theory the current_path would still be available and could forward traffic, but if we mantain a failover_path that's broken or outdated if it tries to switch to it, then it'll pretty much result in a black hole dropping network traffic. We could also consider deactivating in that case too to be extra safe, or maybe we should only allow switching to failover if status_reason doesn't have any failover_path errors? Think about it.

Let me know if it's already covered. From what I followed, we can't just set an empty path for failover_path otherwise it'd leak flows.

[Alopalao]

When a failover_path takes the place of the curren_path, mef_eline checks if is_failover_path_affected_by_link() return True. If the error is in failover_path, it will be treated as that path does not exist so the method will return False. Also, I will add that only deactivates de EVC only when the current_path failed.

[viniarck]

Great

[viniarck]

Also, something to think about is if after a number of attempts on each run if it'll keep trying or give up, and then after that point only a forced reploy or other network event would fix it. It'll depend on how frequently this will run.

[Alopalao]

Good point, I just remembered that we discussed this already. The value will be better if it reflects what needs to be done next, redeploy or delete EVC.

• {"current_path": "Redeploy, {error}"}
• {"current_path": "Delete, {error}"}
• {"failover_path": "Redeploy, {error}"}
• {"failover_path": "Delete, {error}"}

About the consistency check, yes, triggering the check instead will work too. Also, depending in how consistency_check evolves, mef_eline can get away with not keeping deleted EVCs. If the consistency check checks for flows, the cookie and inexistent of an evc should be enough to delete any left-over.

[viniarck]

Got it. Sounds good to me. Let's go for it then. To double check the understanding here, in the case of a deletion here is when indeed it was a user request to delete the EVC but then path removals have failed, right? and redeploy next action will be pretty much for anything else, is that correct?

[Alopalao]

Yes, that is correct

[viniarck]

Consistency check can handle EVC with status_error but the user can do it as well.

That's very interesting. Can we centralize this only for consistency check to be responsible for recoverying fixing it? and then, we can also provide a new endpoint that can schedule a new execution of the consistency check just so users can force/expedite a new run? This is partly also related to this requirement here #114 the original requiement will be covered on epic_mef_eline_consistency_v2 since now consistency check will run for active evcs, but then it could also be triggered on demand right away too. We still don't know how frequent we'll keep the consistency check running, but having a way to run it immediately in moderation seems great. And then in this endpoint, by default it'll run the entire consistency check: running sdn traces and then also checking for status_error|status_reason and executing the actions. If overtime the consistency check gets too heavy, which it probably will, in the endpoint we should also be able to specify which parts it should perform traces|status_reason path fixes. Think about it. Wdyt?

Let's also plan for e2e tests for this new endpoint.

[Alopalao]

Yes, initially I planned for it to be managed by the user with a request. But seeing the requirements for epic_mef_eline_consistency_v2, it looks like we are aligned to have similar solutions in how to deal with any flows left behind. In this epic I am trying to look for the flows left because an error happened and mef_eline has documented. epic_mef_eline_consistency_v2 seems to want to recognize all flows left behind. Creating a request to choose only a part of the consistency check to be scheduled seems great and also can minimize unnecessary load to kytos. Changing it.

[viniarck]

Right. It's up to you and David to decide who will cover this new endpoint if we'll go for it to force a consistency check run, please sync with @Ktmi. Thanks. Let's epic_mef_eline_consistency_v2 further specify how the consistency will run and if it'll still use the same frequency, and then let's consider how it can also run on demand.

Knowing that sdntrace_cp sometimes was taking a few seconds kytos-ng/sdntrace_cp#120 to run in bulk, we'll need to have an interval that's not too agressive also potentially analyze if we'll need to further optimize sdntrace_cp, probably we can get away with it as it is, but eventually we'll need to optimize sdntrace_cp too. Something to keep in mind.

[Alopalao]

This variable was to avoid the look-up for them, a set of failed EVC ids. But I think it will do more harm than help. An EVC can be "fixed" with any changes in topology that affect the EVC, link up|down, disable interface|link|switch. Keeping track on them can be costly. Removing.

[viniarck]

Right. Let's remove then.

[Alopalao]

I also implemented this. The problem is that I was trying to reduce the number of requests of a path to 1. Although this can be done when deleting the EVC entirely or just the current_path, since it only cares about the cookie, it becomes a problem when installing paths. Unless we have a reference about how many flows we have to install in each switch like so:

{
    "switches": [("00:00:00:00:00:00:00:03", 1), ("00:00:00:00:00:00:00:02", 2)]
    "flows": [
            {
                "cookie": 3,
                "priority": 200,
                "instructions": [{
                    "instruction_type": "apply_actions",
                    "actions": [{"action_type": "output", "port": 3}]
                }]
            },
            {
                "cookie": 3,
                "priority": 500,
                "match": {"in_port": 1},
            },
            {
                "cookie": 3,
                "priority": 500,
                "match": {"in_port": 2},
            }
    ],
    "force": true
}

The tuple meaning ({dpid}, {number of flows installed in this switch})

[viniarck]

Got it. Let's go for your initial proposal then, Aldo. POST /api/kytos/flow_manager/v2/flows/bulk, but let's name the endpoint POST /api/kytos/flow_manager/v2/flows_grouped or flows_by_switch, or whichever other name that might reflect more closely this grouping of flows by switch instead of just bulk see what I mean?

Other than that, I was also thinking about the case when force is False, and then only one switch in the middle isn't connected. Right now, currently we write everything to the DB first, so consistency check would also try to send the flow later, which is good. And then in the flow manager response still returning 424 with the dpid in question. And then if users get a 424 error they'll can keep retrying if they expect that the switch will reconnect or just let the consistency eventually deal with it (or if they don't even care bout it just set force True for all switches). I think we can reuse most of the code without significant changes there, but just thinking out loud here too, let me know if you're seeing any other potential issue.

[viniarck]

@Alopalao great proposal and issue discussion.

From my perspective, overall consider it approved +1 from me, you can move on with the implementation parts of where the agreement is clear. Any other parts where there's overlap with consistency check let's also include @Ktmi and make sure we're all on the same page. Thanks, team.