Fix #2785: Support optionally disabling pipelining on AUTH flow

[dbarbosapn]

resolves #2785

This PR is (for now) an initial proposal for fixing the issue mentioned above.
With the new external authentication feature of Envoy, pipelining AUTH with further commands can cause unexpected behavior where we see +OK for AUTH, followed by NOAUTH for the critical trace.

We fix this by adding an optional ConfigurationOptions flag that disables this behaviour.

[mgravell]

So my random idea of a bug in envoy pipelining support was correct? If so, the "correct" fix here should be squarely inside envoy. I'll have a look at this and see if I can find any concerning side-effects, but: they could be subtle.

[dbarbosapn]

So my random idea of a bug in envoy pipelining support was correct? If so, the "correct" fix here should be squarely inside envoy. I'll have a look at this and see if I can find any concerning side-effects, but: they could be subtle.

Envoy supports pipelining but this new feature doesn't since it calls a gRPC dependency (asynchronous) to validate the AUTH result. Would you say it makes more sense to make such call synchronous on Envoy side? I imagine it would have more performance implications... I would guess it would be best for that to be on client-side, which shouldn't be a big issue since it will just await the result of AUTH before proceeding, but doesn't hang any thread.

WDYT?

[mgravell]

It doesn't need to be synchronous per-se, but they can't go and process other commands while it is pending. If the problem is that envoy is implemented using thread-per-connection, then yes: it probably needs to be synchronous or effectively synchronous, since they can't meaningfully process anything else on that connection until the external result is known. So in that context: there's nothing to lose by making it synchronous (or in async terms; they need to await the gRPC call before processing anything else on that same connection).

[dbarbosapn]

It doesn't need to be synchronous per-se, but they can't go and process other commands while it is pending. If the problem is that envoy is implemented using thread-per-connection, then yes: it probably needs to be synchronous or effectively synchronous, since they can't meaningfully process anything else on that connection until the external result is known. So in that context: there's nothing to lose by making it synchronous (or in async terms; they need to await the gRPC call before processing anything else on that same connection).

Thank you! Will investigate on Envoy side and come back with a result on the issue ticket. I'll close the PR for now. Thanks!