Fix shutdown race-condition by introducing a flush_timeout before dropping data

[jleibs]

Resolves: #2556

What

We've always had a race condition during startup where a viewer might not exist despite a client trying to connect to it.

For users of spawn we could have added a bandaid here by probably increasing this sleep:
https://github.com/rerun-io/rerun/blob/main/rerun_py/rerun_sdk/rerun/sinks.py#L180

This wouldn't resolve issues for other use-cases of manually launching a viewer and client without tight orchestration.

Instead, we introduce a timeout (which can optionally be set to None), to use during the disconnected checks when we are flushing. This gives power-users the option to specify None and reduce risk of losing data (at an increased risk of blocking during flush when things go wrong).

This PR also bumps a couple of debug logs up to warnings to make it more clear when data is being dropped.

Checklist

• I have read and agree to Contributor Guide and the Code of Conduct
• I've included a screenshot or gif (if applicable)
• I have tested demo.rerun.io (if applicable)

• PR Build Summary
• Docs preview
• Examples preview

[Wumpf]

looks very promising!
Gave it only a quick spin and seems fine. Maybe @abey79 wants to check as well?

[Wumpf]

that timeout should be at max as big as self.disconnected_timeout, no?

[jleibs]

Not exactly. The other timeout is specific to behavior when flushing whereas this is specific to the tcp connection giving up during normal operation. Changing this to something low could have adverse affects on behavior of the system when streaming over wifi.

[abey79]

This appears to work well on my side 👍

Should we remove the time.sleep(0.5) in spawn()? I would think so, and certainly don't need it to have a successful spawn & connect. The major side effect is that this warning arises more systematically:

[2023-07-26T09:18:55Z WARN  re_sdk_comms::buffered_client] Failed to send message: Failed to connect to Rerun server at 127.0.0.1:9876: Connection refused (os error 61)

This is a false positive though, as one of the retry clearly goes through. Maybe we should emit a warning only after exhausting all the retries, and list all the error encountered when attempting connection.

For example:

Failed to connect to Rerun server at 127.0.0.1:9876 after 3 retries: Connection refused (os error 61) (3 times)

or

Failed to connect to Rerun server at 127.0.0.1:9876 after 3 retries: Connection refused (os error 61) (1 time), Incompatible viewer version (2 times)

(I'm making that last one up ofc.)

[Wumpf]

Should we remove the time.sleep(0.5) in spawn()

➕

[jleibs]

Should we remove the time.sleep(0.5) in spawn()

I would prefer not to remove remove this sleep until we are properly waiting for the sender to be up because otherwise we are always going to get failed connections in the debug log.

I will add some different book-keeping on the logging though to only warn after 3 attempts either way to get rid of the spurious log for folks where 0.5 isn't quite long enough.