stub: support restart after ttrpc connection lost

[zhaodiaoer]

In some scenarios such as runtime restart or the occurrence of nri request timeout, the ttrpc connections between the plugin and the runtime will be actively closed by the runtime, even the underlying network connection will be closed together. After this, the plugin must need to re-register to the adaptation side, but now the stub object cannot be reused for this; if the running plugin wants to reconnect to the runtime, the only way is to create a new stub for the plugin.

This commit introduce a new stub option WithAutoRestartDelay. The plugin developer can use this to build a reuseable stub, which will wait for a confiurable time duration and restart automatically after the ttrpc connection is lost, then plugin will be auto-reregistered to the adaptation.

[zhaodiaoer]

@klihub @fuweid @mikebrow can anyone help to review this ? thanks.

[klihub]

@klihub @fuweid @mikebrow can anyone help to review this ? thanks.

My first question is that do we really want to initiate a reconnection from the stub itself ? Instead of letting some higher-level application logic decide if, and when, attempting a reconnection is both desirable and safe ?

We have the WithOnClose() stub option which should allow applications to detect when a connection to the runtime is gone and then trigger at will a reconnection on their own accord.

[zhaodiaoer]

@klihub @fuweid @mikebrow can anyone help to review this ? thanks.

My first question is that do we really want to initiate a reconnection from the stub itself ? Instead of letting some higher-level application logic decide if, and when, attempting a reconnection is both desirable and safe ?

We have the WithOnClose() stub option which should allow applications to detect when a connection to the runtime is gone and then trigger at will a reconnection on their own accord.

I quite approve of the idea of leaving the reconnection work to the higher-level application to handle. However, in the actual attempt, I discovered the following problems that are difficult to solve by the higher-level application logic. Most of them come from the current internal logic of the stub:

1. When the initial stub.conn is closed, it is not nil and no any logic will reset it to nil. Therefore, stub.connect() will do nothing. The multiplex required for reconnection will be constructed base the old stub.conn (the ttrpc connection stage comes after multiplex construct). Here, it is required that stub.conn is a stdnet.Conn implementation that is still usable after being closed. Maybe there are some cunning ways to achieve this, but it is inconsistent with the semantics defined by the stdnet.Conn interface.
2. The handler provided by WithOnClose() cannot take stub object, so it cannot operate stub.conn directly, only can send some notify to other functions to do reconnection work.
3. Last minor problem: just like the stub.conn variable mentioned in the 1st problem, once stub.started is set to true, there is no chance to reset it to false, new multiplex and new ttrpc connections will not be created.

To summarize the problems described above, most of the current connection handling logic is defined within the stub, and there are not enough exposed public methods to implement custom reconnection logic. If my understanding is incorrect, or if there are currently any proposals for optimizing the design of the plugin <-> runtime connection management, please let me know. I am willing to implement the code for these designs. But before that, if a convenient reconnection handling option can be provided to plugin developers for easy use, it might also be a good choice.

[klihub]

When the initial stub.conn is closed, it is not nil and no any logic will reset it to nil. Therefore, stub.connect() will do nothing

Sorry, I was not specific enough. I did not mean to try reusing/reconnecting the already disconnected stub. I meant to decide/check whether and when re-establishing the connection is safe, then create a new stub and connect that one...

[zhaodiaoer]

When the initial stub.conn is closed, it is not nil and no any logic will reset it to nil. Therefore, stub.connect() will do nothing

Sorry, I was not specific enough. I did not mean to try reusing/reconnecting the already disconnected stub. I meant to decide/check whether and when re-establishing the connection is safe, then create a new stub and connect that one...

oh i got you, In fact, currently we do achieve the purpose of reconnection and re-registration plugin by rebuilding the stub object.

I think a rather crucial point here is what the long-term positioning of the stub is like. Does it only cover the lifecycle of a single connection or the lifecycle of the plugin? If it is an external type of plugin, it is a relatively common requirement for its operation cycle to cover multiple connections. Rebuilding the entire stub object every time the connection is disconnected is a bit counterintuitive.

[klihub]

When the initial stub.conn is closed, it is not nil and no any logic will reset it to nil. Therefore, stub.connect() will do nothing

Sorry, I was not specific enough. I did not mean to try reusing/reconnecting the already disconnected stub. I meant to decide/check whether and when re-establishing the connection is safe, then create a new stub and connect that one...

oh i got you, In fact, currently we do achieve the purpose of reconnection and re-registration plugin by rebuilding the stub object.

I think a rather crucial point here is what the long-term positioning of the stub is like. Does it only cover the lifecycle of a single connection or the lifecycle of the plugin?

Well, (be)for(e) answering that question there is a related question to consider: 'what is a plugin ?' or more precisely 'can the plugins lifecycle be different from the lifecycle of the connection of the plugin ?'.

These might sound very philosophical at first, outright ridiculous even, before you start to consider things from the runtime's perspective. On the client side, sure, there is a clear difference between the lifecycles of the plugin and its connection. On the runtime side however, there isn't a clear distinction. Once a connection is closed/lost, the runtime considers the plugin gone forever. If the plugin was an internal one, we would need to restart it to get it connected, up, and running again. That creates a new connection, a new process, and a new struct for the plugin. Clearly a new instance from all aspects. If it was an external plugin, even if it does not exit but reconnects from within the same process, a new connection and a new internal struct is created in the runtime. Again, a new instance from the runtime's point of view. So from the runtime's perspective the lifecycles of the connection and the plugin are the same...

If it is an external type of plugin, it is a relatively common requirement for its operation cycle to cover multiple connections. Rebuilding the entire stub object every time the connection is disconnected is a bit counterintuitive.

That is a fair and valid point. I'd still argue that we should make the stub restartable and reconnectable by the plugin, instead of trying to make the stub restart and reconnect itself.

Considering that it is completely unpredictable in what context a 'connection lost' event comes and in what state the plugin itself is (as opposed to the stub) at that moment, only the application can know when it is safe to reconnect and, for instance, what kind of locking or other synchronization need to be taken care of before this can be attempted.

So IMO, we should reset/clear enough of the internal state of the stub that it should be possible to safely restart/reconnect it. And maybe this should always be done explicitly... perhaps only by an explicit call to Stop() ?

So then for a reconnectable plugin

• high level plugin logic would need to install an OnClose() handler to notice lost connections
• arrange it so that the stub is Stop()ped once connection is lost (not necessarily from the handler)
• Stop() would cause the stub to reset enough of its internal state
• eventually the plugin would call Start() on the stub again to reconnect, once this is safe to do

[zhaodiaoer]

So then for a reconnectable plugin

• high level plugin logic would need to install an OnClose() handler to notice lost connections
• arrange it so that the stub is Stop()ped once connection is lost (not necessarily from the handler)
• Stop() would cause the stub to reset enough of its internal state
• eventually the plugin would call Start() on the stub again to reconnect, once this is safe to do

Hi! Klihub, thank you very much for your explanation! After these discussion, I think I understand your concern, that is, it's okay to make the stub on the client side restartable, but it's unsafe to let it restart "automatically".

At present, should I first modify the current patch to achieve “Stop() would cause the stub to reset enough of its internal state”, I think this is a good start.

[klihub]

So then for a reconnectable plugin

• high level plugin logic would need to install an OnClose() handler to notice lost connections
• arrange it so that the stub is Stop()ped once connection is lost (not necessarily from the handler)
• Stop() would cause the stub to reset enough of its internal state
• eventually the plugin would call Start() on the stub again to reconnect, once this is safe to do

Hi! Klihub, thank you very much for your explanation! After these discussion, I think I understand your concern, that is, it's okay to make the stub on the client side restartable, but it's unsafe to let it restart "automatically".

At present, should I first modify the current patch to achieve “Stop() would cause the stub to reset enough of its internal state”, I think this is a good start.

Yes, I also think that would be the best to start with.