Timeout handling at the client side.

[batiati]

The problem

The VSR client handles request timeouts by retrying indefinitely through each node of the cluster until it finds one that can reply:

From vsr/client.zig

 fn on_request_timeout(self: *Self) void {
	 
          // Omitted ...
	 
	 // We assume the leader is down and round-robin through the cluster:
	 self.send_message_to_replica(
	 @intCast(u8, (self.view + self.request_timeout.attempts) % self.replica_count),
	 message,
	 );
	 }

In this approach, the client implementation does not offer the application semantics for "timed out request" or "disconnected client", resulting in potentially endless wait if the client loses connectivity or if the cluster itself goes down.

Since none of the ongoing requests ever finish in this condition, the C tb_client's exhausts its max concurrency limit quickly, causing the client to "hang" waiting for resources before submitting a new request.
Also, both the Java client and the Go client would block if close is called in this situation, as long it waits for all ongoing requests to end before closing.

Solution

First, I'd like to confirm if this is indeed an issue or if it was a design decision:

1. Being it an issue, I suggest adding a new configuration for "the max number of times we should round-robin through the cluster" and after that, drop the message and callback the client with a new TimedOut error.

2. Being a design choice, it makes sense since the client is part of the cluster and isn't supposed to drop and give up any message on purpose. In this case, we may implement a way to notify the application that the client is disconnected, exposing that a) the application can submit no more requests; and b) ongoing requests may or may not complete and must be checked by the application once the client reconnects.

(...)

Please, feel free to point out a better solution for this case, I'll be happy to send a PR addressing it.

[nurturenature]

From the experience of writing a small test app using the Java client, there are some behaviors that would be nice:

new Client()

• returns an error if it's known that a client can not be created, was not created
• then may block indefinitely as it tries, until it knows

client.close()

• may not block
• may return not able to close due to reason

client.status()

• may not block
• returns summary status
• may return unknown

client.lookupAccounts/createTransfers()

• may block indefinitely

A current example, asking for max-client + 1 should return an error vs crashing the app and the JVM:

error(client): 327448509614244119477983598218977414147: session evicted: too many concurrent client sessions
thread 34600 panic: session evicted: too many concurrent client sessions

In general its OK for the app to be responsible for timeouts.
A well designed app will already account for maybe the message was delivered, maybe it was acted on, maybe it was replied to.
But it should always be OK for the app to ask for something new or its status.

[batiati]

@nurturenature, once again, thank you for the detailed report!

There are a few situations where we can @panic on the client side, causing the application to crash, but this one you reported should not panic, I will open an issue for that ASAP.

thread 34600 panic: session evicted: too many concurrent client sessions

[sentientwaffle]

Hi @batiati, I apologize for the slow reply.

The lack of a timeout in the client is a design choice. The issue is that even if we time out, the request may have already made it to the cluster and been executed, and the client hasn't received a reply due to an asymmetric network partition. Alternatively, the request (or reply) packet may simply be delayed but arrive eventually.

If the client surfaces a timeout error, not only does it appear to the developer as if their request did not (and will not) arrive, which is misleading, but their application loses the ability to receive the reply to that (very slow) request. As implemented right now, we will keep retrying — eventually the cluster (or network) will be back, and will handle the request.

Being a design choice, it makes sense since the client is part of the cluster and isn't supposed to drop and give up any message on purpose. In this case, we may implement a way to notify the application that the client is disconnected

The issue with "notify[ing] the application that the client is disconnected" is that we can never really know that the client is disconnected, we can only guess.

ongoing requests may or may not complete and must be checked by the application once the client reconnects

This seems like it would be extremely difficult for the application developer to implement correctly.

We are going to let this simmer to explore possible solutions. Thanks for surfacing this issue.

[batiati]

Hi @sentientwaffle, thank you for the clarification.

That makes perfect sense to me.
At first glance, it seems odd from the application's perspective, but it is fine if we just assume this behavior for network partition events.

Another point I'd like to address is the @panic caused by session eviction. What do you think we should do in this case? I suppose that it is ok to panic the user's application on failed asserts, but I'm not sure about this one.

[sentientwaffle]

Another point I'd like to address is the @panic caused by session eviction. What do you think we should do in this case? I suppose that it is ok to panic the user's application on failed asserts, but I'm not sure about this one.

Yes, this panic will go away. In this case we do need to surface an error from the client.

[batiati]

Yes, this panic will go away. In this case we do need to surface an error from the client.

Nice, I'll work on that in a separate PR.
Thanks!

[jorangreef]

Thanks @batiati, closing for the time being. We have this in mind.

[aphyr]

I suggest re-visiting this issue. TigerBeetle promises to make indefinite errors (like timeouts) impossible. This is not true: TigerBeetle clients naturally produce timeout errors, and users can easily wrap TigerBeetle in their own timeouts. Moreover, by flattening all errors into delays, TigerBeetle clients turn definite failures into indefinite ones. This exacerbates the safety problem @sentientwaffle notes, and leaves users with less choice about how to handle faults.

First, TigerBeetle clients naturally produce timeout errors. The Java client's async functions, like createTransfersAsync, sensibly return CompletableFutures. CompletableFuture is intended to represent operations with indefinite failures, like network requests. Timeouts are deeply woven into this datatype: one awaits them using .get(timeout, timeUnit), or wraps them in .orTimeout(seconds, timeUnit) to throw a timeout automatically. The .Net client's createTransferAsync and friends return Task objects which similarly offer timeout-driven Wait() functions.

Even if users constrain themselves to unbounded blocking calls, applications rarely have unbounded runtimes. It seems likely that applications will wrap TigerBeetle calls in their own timeouts. If they do not, the application may eventually terminate, which is a worse kind of indefinite failure. Even when applications can wait, their clients, or the human beings waiting for an operation, may give up at any time. The challenge of indefinite errors is intrinsic to asynchronous networks and cannot be eliminated.

TigerBeetle has one technique for handling failure: a silent internal retry mechanism. This unnecessarily converts definite errors into indefinite ones. For example, imagine a common fault: a TigerBeetle server has crashed. The application makes a createTransfer request. The client attempts to open a TCP connection and receives ECONNREFUSED. The client knows internally that this request can not possibly have executed: it has a definite failure. However, it refuses to inform the caller, and instead retries again and again. The caller's only sign of an error is that the client appears to have stalled. When the caller times out or eventually shuts down, that definite failure becomes indefinite. Instead of making indefinite errors impossible, TigerBeetle's client design proliferates them.

Instead of hiding information about the nature of failures, TigerBeetle should expose it. Clients should define a set of errors, return them to callers when problems occur, and clearly declare which are indefinite versus definite.

It is perfectly fine to keep automatic retries--perhaps even unbounded ones--but this behavior should be parameterized. TigerBeetle clients should take options controlling (at least) the maximum time allowed for opening a connection, and for awaiting responses from a submitted request. Users can request unbounded timeouts if they choose.