Add Windows platform compatibility #4203
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds Windows support through conditional compilation, platform-specific alternatives, and graceful degradation for features without Windows equivalents. ## Signal Handling (server/src/signal.rs) - Split into platform-specific modules using #[cfg(unix)] / #[cfg(windows)] - Unix: Retains full signal handling (SIGTERM, SIGINT, SIGUSR1, SIGUSR2, SIGHUP) - Windows: Uses Ctrl+C for shutdown; other signals become no-ops (pending futures) ## Unix Domain Sockets - Guard all UDS code with #[cfg(unix)] throughout the codebase - Add #[cfg(not(unix))] match arms that return errors or panic for UDS paths - Files affected: - crates/types/src/net/listener.rs - Added AcceptedStream enum abstraction - crates/types/src/net/address.rs - Guard UDS parsing - crates/core/src/network/net_util.rs - Guard UDS server binding - crates/core/src/network/grpc/connector.rs - Guard UDS channel creation - crates/hyper-uds/src/lib.rs - Made entire module Unix-only - cli/src/clients/admin_client.rs - Guard reqwest unix_socket usage - cli/src/cli_env.rs - Guard UDS path handling - crates/local-cluster-runner/src/node/mod.rs - Guard UDS client usage ## AcceptedStream Abstraction (crates/types/src/net/listener.rs) - Created unified AcceptedStream enum implementing AsyncRead + AsyncWrite - Allows single accept() method signature across platforms - Eliminates duplicated stream handling code in callers - Simplifies crates/core/src/network/net_util.rs and crates/ingress-http/src/server.rs ## Kafka Feature (Optional) Made Kafka integration optional to avoid rdkafka/OpenSSL build issues on Windows: - crates/worker/Cargo.toml - Added kafka and kafka-oidc features - crates/worker/src/lib.rs - Feature-gated kafka imports and usage - crates/node/Cargo.toml - Added kafka and kafka-oidc features - server/Cargo.toml - Added kafka and kafka-oidc features (default on) Windows users should build with: --no-default-features -F no-trace-logging ## Resource Limits (rlimit) - Guard rlimit::increase_nofile_limit() calls with #[cfg(unix)] - Made rlimit a Unix-only dependency in: - server/Cargo.toml - benchmarks/Cargo.toml - tools/bifrost-benchpress/Cargo.toml - tools/restatectl/Cargo.toml - crates/local-cluster-runner/Cargo.toml ## Process Management (local-cluster-runner) - Guard nix crate usage with #[cfg(unix)] - Added Windows alternatives using std::process and taskkill - Platform-specific process group and signal handling - Made nix a Unix-only dependency ## Build Scripts - crates/types/build.rs - Handle missing git on Windows - crates/service-protocol-v4/build.rs - Handle missing git on Windows ## Test Infrastructure - Guard Unix-specific test addresses with #[cfg(unix)] - Add Windows-compatible alternatives using TCP addresses - Files: nodes_config.rs, balanced_spread_selector.rs, checker.rs, nodeset_selector.rs, connection_manager.rs, test_util.rs ## Hakari Configuration - Excluded rdkafka and rdkafka-sys from workspace-hack to prevent accidental inclusion on Windows builds ## Feature Parity | Feature | Unix | Windows | |----------------------|----------------|----------------------| | HTTP/TCP networking | Full | Full | | Unix Domain Sockets | Full | Error (unsupported) | | Graceful shutdown | SIGTERM/SIGINT | Ctrl+C | | Config dump (SIGUSR1)| Full | Not available | | DB compact (SIGHUP) | Full | Not available | | jemalloc | Full | System allocator | | Kafka ingress | Full | Requires manual deps |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
AcceptedStreamabstraction for cross-platform stream handlingBuilding on Windows
Kafka with OIDC support (via rdkafka) depends on krb5-src (Kerberos), which doesn't build on Windows - its build script requires Unix tools (
cat,sed). Rather than require Unix users to change their build process, I made Kafka opt-out for Windows users:Existing Unix builds require no changes -
cargo buildcontinues to work as before with Kafka enabled by default.Changes
Signal Handling
server/src/signal.rsinto platform-specific modulesUnix Domain Sockets
#[cfg(unix)]AcceptedStreamenum implementingAsyncRead + AsyncWritefor unified stream handlingKafka Feature
kafkaandkafka-oidcfeatures--no-default-featuresResource Limits
rlimitcalls with#[cfg(unix)]rlimitandnixcrates Unix-only dependenciesTest Infrastructure
Listeners::new_tcp_listener()for cross-platform test setuptest_http_posttest that runs on all platforms#![cfg(unix)]Feature Parity
Test plan
cargo test -p restate-types --no-default-features- 196 passcargo test -p restate-core --no-default-features- 10 passcargo test -p restate-ingress-http --no-default-features- 25 passNote: Pre-existing test isolation issues
During testing I discovered some pre-existing test isolation bugs unrelated to this PR. When running test suites together, some tests fail due to global
ConfigurationandTaskCenterstate leaking between tests. All tests pass when run individually. Examples:restate-bifrost:test_append_record_too_largesets a 5-byte record limit that persists to subsequent testsrestate-types:endpoint_manifest_options_propagation::*tests fail with incorrectjournal_retentionvaluesrestate-core:test_basic_lifecyclefails when run after other tests due to TaskCenter stateThese issues don't appear to be related to my changes.