[Epic] Improve the usability and reliability of SSH TUNNELs

[guswynn]

Product Outcome

SSH tunnel connections are resilient and observable

Context

Over the past few weeks, its become clear there are usability, reliability, and debuggability issues with SSH TUNNELs, particularly when used with kafka. This issue tracks a set of improvements that can be made SSH TUNNELs:

Tasklist

Reliability

• storage: increase reliability of ssh connections #18371
  • releasing in 0.48
• storage: improve reliability of SSH tunnels with large clusters #18547
• Ssh tunnels are possibly transiently broken on macos:
  • Fixed cargo: update signal-hook-registry to prevent a weird bug #18437
• Ssh tunnel connections transiently panic, at least in ci
  • https://buildkite.com/materialize/tests/builds/52408#018724a9-8ece-4afa-affc-319bc1d475e5
  • this assert is hit: https://github.com/openssh-rust/tokio-io-utility/blob/main/src/async_read_utility/inner.rs#L206
  • Filed upstream as: SSH connection occasionally panics with IO assertion openssh-rust/tokio-io-utility#17
  • storage: update tokio-io-utility #18591
• ssh connections are mysteriously broken with "the connection was terminated" in production
  • This is a problem with the control master
• storage: errors while connecting to CSR will crash clusterd #19252

Usability

• Newly advertised kafka brokers aren't routed through tunnels - Add default SSH TUNNEL option for Kafka connections #18375
  • Fixed in storage: support top-level ssh tunnels for kafka #18454
• [Much lower priority] Harden tunnels by supporting multiple bastions
  • https://www.notion.so/materialize/SSH-tunnel-hardening-e4a4add751754876b32a65c3cf73a714

Debuggability (and documentation)

• Clean up storage networking tuning tech debt #18683
• Broken connections to ssh aren't not communicated to users (related to storage: audit style and content of errors #17826)
  • Source pipeline creation errors are panics, and not routed to mz_source_status_history
    • storage: source reader failures shouldn't panic #18491
  • mz_source_statuses doesn't communicate whether or not the errors reported from rdkafka have ssh as an underlying cause
    • storage: broadcast ssh tunnel errors to sources and sinks #18616
      • afterwards, enrich errors with rdkafka callbacks
      • add connection status polling to postgres sources
      • Consider checking the status at a higher-level: storage: broadcast ssh tunnel errors to sources and sinks #18616 (review)
  • ssh tunnel errors don't communicate the underlying cause of the broken pipe
    • This is particularly helpful when the user forgets to authorize the tunnel keys
    • storage: improve the detail of errors around kafka+ssh that are logged #18517
• How to debug ssh connection errors needs to be documented better
  • docs: Check if SSH bastion is forwarding requests #18489

[benesch]

Harden tunnels by supporting multiple bastions

I'd propose skipping this one for now! It's a big lift to make it work reliably (how do you detect if a connection has stalled and needs to be restarted in the other SSH bastion?), and I think we have a good workaround (asking customers to ping their SSH bastions and restart them if they fail).

[guswynn]

@benesch I mentioned the order of priority here: https://materializeinc.slack.com/archives/C04DJT084KA/p1680117095320179, ill transfer this context to the issue!

[benesch]

Awesome, thanks!

[sentry-io]

Sentry issue: DATABASE-BACKEND-2P2

[benesch]

Wanted to record some thoughts from debugging SSH tunnels over the weekend:

• We need a more principled take on when and how to report SSH tunnel errors. Right now we'll report an initial SSH tunnel error, but not any future SSH tunnel errors.
  • Chatted with Gus about this a bit today. There's a perhaps not so bad path to improving this, starting by consistentizing Kafka errors. Everywhere we report a Kafka error, we should a) use a metadata fetch request and report only the errors from that API, and b) always prefer to report any SSH tunnel error rather than a Kafka error.
• The reported error when a PostgreSQL connection fails is terrible (often just "failed to connect to remote host" regardless of whether the SSH tunnel failed, or the SSH credentials were wrong, or the PostgreSQL server was down, or the PostgreSQL credentials were wrong). AFAICT, the error message details are available, but we store the error in PlanError::FetchingPostgresPublicationInfoFailed, which embeds a PostgresError::Generic, which embeds an anyhow::Error, which contains "failed to connect to remote host" as the top-level error, with the true cause embedded within. Convincing this error to print its causal chain is REALLY hard. You can't convert Arc<PostgresError> to an anyhow::Error, which means you can't use anyhow's support for printing causal chains. I think we need a rule like "you can never put an anyhow::Error instead another error type; you can only use them at the top level" since otherwise it all falls apart.

[aljoscha]

yes! There is #17826, which we still need to flesh out

[aljoscha]

* The reported error when a PostgreSQL connection fails is terrible (often just "failed to connect to remote host" regardless of whether the SSH tunnel failed, or the SSH credentials were wrong, or the PostgreSQL server was down, or the PostgreSQL credentials were wrong). AFAICT, the error message details _are_ available, but we store the error in [`PlanError::FetchingPostgresPublicationInfoFailed`](https://github.com/MaterializeInc/materialize/blob/bc93581a3680371a8f51345284a6674003823d76/src/sql/src/plan/error.rs#L110), which embeds a `PostgresError::Generic`, which embeds an `anyhow::Error`, which contains "failed to connect to remote host" as the top-level error, with the true cause _embedded within_. Convincing this error to print its causal chain is REALLY hard. You can't convert `Arc<PostgresError>` to an `anyhow::Error`, which means you can't use anyhow's support for printing causal chains. I think we need a rule like "you can never put an anyhow::Error instead another error type; you can only use them at the top level" since otherwise it all falls apart.

I think we can actually print the chain of errors, see #18583. We might have to do some more work to ensure that all the errors are actually there.

For PostgresError::Generic this works because the variants are all #[error(transparent)] so we get the Display impl of the wrapped error. @benesch I think your example uses PostgresError::Ssh or PostgresError::SshIo, for which this doesn't work. We'd have to write a custom Display impl for PostgresError to get those to work nicely, I'm afraid (typing that up quickly, to see).

[benesch]

Closing as this is now complete enough!