Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support UDS listen address for tokio-console #16537

Merged
merged 2 commits into from
Dec 11, 2022

Conversation

teskje
Copy link
Contributor

@teskje teskje commented Dec 9, 2022

This PR fixes local tokio-console support, which was broken in ac1028d because tokio-console does not support Unix domain sockets (yet, see: tokio-rs/console#388).

The fix involves adding that support by switching to @benesch's fork of concole-subscriber and using the mz_ore::netio::SocketAddr type for the tokio-console listen address.

It is now also required to use the forked version of the tokio-console CLI client. The developer docs are updated to reflect this.

Motivation

  • This PR fixes a previously unreported bug.

Running environmentd with tokio-console support enabled is broken:

environmentd: 2022-12-09T12:23:53.131943Z  INFO mz_orchestrator_process: launching compute-cluster-s1-replica-2-0 via --controller-listen-addr=/var/folders/30/xdglsm2j3tz1rn1n7yygtm7c0000gn/T/3936d1d75b734cbabb3d0678b275c3deafe82812 --internal-http-listen-addr=/var/folders/30/xdglsm2j3tz1rn1n7yygtm7c0000gn/T/c489d7731630ec1fa5fb8ddba4c0da9dd9c9eafb --opentelemetry-resource=instance_id=s1 --opentelemetry-resource=replica_id=2 --log-filter=info --log-format=text --log-prefix=compute-cluster-s1-replica-2 --tokio-console-listen-addr=/var/folders/30/xdglsm2j3tz1rn1n7yygtm7c0000gn/T/1209e7e52cdc107ee25ca4998196aa23922fb9e4 --tokio-console-publish-interval=1000000 us --tokio-console-retention=3600000000 us --pid-file-location=/private/var/folders/30/xdglsm2j3tz1rn1n7yygtm7c0000gn/T/environmentd-local-az1-2ba6f456-fae2-4c9b-a8df-8f398ee4cb82-0/compute-cluster-s1-replica-2/0.pid --secrets-reader=process --secrets-reader-process-dir=/Users/jan/devel/materialize/mzdata/secrets...
error: Invalid value "/var/folders/30/xdglsm2j3tz1rn1n7yygtm7c0000gn/T/d8309b97c7d54152fe0a4ded500c397714d4b559" for '--tokio-console-listen-addr <TOKIO_CONSOLE_LISTEN_ADDR>': invalid socket address syntax

For more information try --help
environmentd: 2022-12-09T12:23:53.149669Z ERROR mz_orchestrator_process: compute-cluster-s2-replica-3-0 exited: ExitStatus(unix_wait_status(512)); relaunching in 5s

Checklist

This commit fixes local tokio-console support, which was broken in
ac1028d because tokio-console does not support Unix domain sockets
(yet).

The fix involves adding that support by switching to Nikhil's fork of
concole-subscriber and using the `mz_ore::netio::SocketAddr` type for
the tokio-console listen address.
@teskje teskje marked this pull request as ready for review December 9, 2022 13:18
@teskje teskje requested review from benesch and guswynn December 9, 2022 13:18
Copy link
Member

@benesch benesch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @teskje! LGTM. I pushed up a slight modification to move the Git reference to a MaterializeInc fork, since the uds branch is associated with a PR that's still undergoing review, and I might force push at it. We can keep the MaterializeInc fork stable though.

@benesch benesch enabled auto-merge (rebase) December 11, 2022 20:38
@benesch benesch merged commit 3c194ef into MaterializeInc:main Dec 11, 2022
@teskje teskje deleted the tokio-console-uds branch December 12, 2022 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants