Simplify healing - determinate reselect state for whole chain about sdk

[d-uzlov]

Original long proposal with more reasoning

Current behavior

From what I can understand, current behavior with trying to restore the old connection on reselect is an attempt to prevent resource leaks.

If Close call goes through all apps in the connection path, then all elements clear data about connection, and a new request is treated as a completely new request, with proper reselect.
If Close call doesn't reach some apps, then reselect request is treated as a refresh.
If Close call didn't reach some app and the app has restarted, then we try to restore the connection, based on assumption that this is a refresh request call.

We can change a reselect request in some way to properly indicate that this is a reselect request.
This will solve the issues with lost Close case. It will not matter is Close reached all apps in the path or not, all apps will treat the request as reselect.

There is an issue with this approach: if the Close didn't reach the app and we create a new connection instead of restoring the old one, the app will leak resources.

If the reselect request goes through the same path, then we can try to detect the reselect and try to clear resources.
If reselect request goes through a different path, then there is not much we can do during the request.

Open questions

1. How can a server understand that some request is a reselect?
2. Should we try to clear resources on a reselect after failed Close? How?

Second question is the hard one. If we decide to ignore it, the first one has several easy solutions.
If we ignore it, resources will be cleared eventually, using the timeout mechanism.

Option 1 for reselect

Simply clear connection path on reselect (except for the first path segment).
Then updatePath will generate a new IDs for all path segments, and all chain elements will treat the request as a completely new connection.

sdk/pkg/networkservice/common/begin/event_factory.go

Line 107 in 6fa2f68

request.GetConnection().Mechanism = nil

request.GetConnection().Path.PathSegments = []*networkservice.PathSegment{request.GetConnection().Path.PathSegments[0]}

Pros:

• No changes in other chain elements needed.

Cons:

• Reselect looks like a new request
• Doesn't solve resource leaks
• Current attempt to prevent resource leaks will no longer work

Option 2 for reselect

Add a new reselect field to request.

https://github.com/networkservicemesh/api/blob/5d372416a0b4b0d70d37a801e6d24fe37cf5c432/pkg/api/networkservice/networkservice.proto#L28

message NetworkServiceRequest {
  connection.Connection connection = 1;
  repeated connection.Mechanism mechanism_preferences = 2;
  bool reselect = 3;
}

...
if request.reselect {
    clearOldResources()
}
...

Pros:

• Distinct reselect (an ability to clear resources)

Cons:

• Requires changes in all chain elements with state
• Change in API
• Doesn't help to clear resources if we go through a different path on reselect

Proposal to clear resources

Currently resources are linked to the connection ID. But updatePath element changes connection ID to ID of current path segment.
If path segment ID changes (which will happen if we clear the path), resources will leak.

We can try to use a more stable ID for allocating and clearing resources.
Particularly, the first path segment should never change on refresh or reselect requests.
ID + NS-name should be a unique key.
Then we will be able to identify previous connection, even if path segment ID has changed.

This will obviously require changes in all chain elements that allocate resources themselves, similar to "Option 2".

[d-uzlov]

Current behavior

If Close reaches all apps in path, it clears all caches, and reselect works as expected.

If Close doesn't reach some apps, those apps try to restore old connection to avoid resource leaks.
Therefore, reselect doesn't work.

Solution 1

Changes:

• Clear request path.

When path is empty, updatePath will create new IDs.
All the caches that prevent reselect from working are linked to path segment ID.
With new ID we are guaranteed to get reselect.

Pros:

• Very simple
• Fully backward-compatible

Cons:

• The only way to clear leaked resources is using timeout

Solution 2 (Best solution)

Changes:

• Create a new state of connection: RESELECT_REQUESTED
  • https://github.com/networkservicemesh/api/blob/5d372416a0b4b0d70d37a801e6d24fe37cf5c432/pkg/api/networkservice/connection.proto#L33-L37
• Set this state in event factory when using reselect
• Add a new chain element, reselectcleanup, that will request Close if it sees RESELECT_REQUESTED on an existing connection

If Close from client has reached all apps in the connection path, everything will work as before.
If local Nsmgr has restarted, it's likely we go to the same forwarder, and forwarder will close the old connection.
If local forwarder has restarted, but we go to the same remote node, the remote forwarder or nsmgr will clear resources.

It is still possible to have resource leaks if we can't reach some apps in the path.
But this solutions will fix many common cases.

Pros:

• "Best effort" resource cleanup
• Still simple

Cons:

• A small change in API

Example when NSMgr restarted: