TalaatHarb commented on issue #4037:
URL: https://github.com/apache/iggy/issues/4037#issuecomment-5522486138
I tried implementing the Windows-native build fixes locally, based on the
blockers documented above. The result is promising: `iggy-server.exe` now
builds successfully on Windows/MSVC (both debug and release builds) after a
small set of portability changes plus the vcpkg `hwloc` setup.
I was able to pass:
```powershell
cargo check -p journal -p message_bus -p server --all-targets
cargo test -p journal truncate --lib
cargo test -p message_bus --lib fd_transfer
cargo test -p message_bus --lib socket_opts
cargo test -p server --lib segment_recovery
cargo build -p server --bin iggy-server
cargo build -p server --bin iggy-server --release
.\target\release\iggy-server.exe --help
```
One local vcpkg quirk was still needed: `hwloc.pc` emits `-lhwloc`, which
MSVC tries to resolve as `libhwloc.lib`, while vcpkg installs `hwloc.lib`. I
worked around that locally with:
```powershell
Copy-Item D:\tools\vcpkg\installed\x64-windows\lib\hwloc.lib `
D:\tools\vcpkg\installed\x64-windows\lib\libhwloc.lib
```
That part probably deserves either documentation or a build-script-side
workaround. (there are probably some interesting documentation changes needed
in general for that feature)
## Code changes that made the Windows build work
### 1. `journal::file_storage`: use Windows handles instead of Unix fds
`core/journal/src/file_storage.rs` used Unix-only `std::os::fd::AsFd`. On
Windows, the equivalent is `std::os::windows::io::AsHandle`.
Suggested shape:
```rust
#[cfg(unix)]
use std::os::fd::AsFd;
#[cfg(windows)]
use std::os::windows::io::AsHandle;
```
And in `truncate()`:
```rust
#[cfg(unix)]
let file = fs::File::from(file.as_fd().try_clone_to_owned()?);
#[cfg(windows)]
let file = fs::File::from(file.as_handle().try_clone_to_owned()?);
```
This keeps the same behavior: duplicate the already-open compio file handle,
then use blocking `std::fs::File::set_len()` + `sync_all()` for
recovery/truncation.
### 2. `message_bus::fd_transfer`: move from raw Unix fds to
`socket2::Socket`
The current implementation manually duplicates raw Unix fds using:
```rust
libc::fcntl(original, libc::F_DUPFD_CLOEXEC, 0)
```
That has no Windows equivalent. `socket2::Socket::try_clone()` already
provides the cross-platform behavior needed here:
- Unix: uses `F_DUPFD_CLOEXEC`
- Windows: uses non-inheritable Winsock socket duplication
Suggested direction:
```rust
use socket2::{SockRef, Socket};
pub struct DupedFd(Socket);
pub fn dup_fd(stream: &TcpStream) -> io::Result<DupedFd> {
SockRef::from(stream).try_clone().map(DupedFd)
}
pub fn wrap_duped_fd(fd: DupedFd) -> io::Result<TcpStream> {
TcpStream::from_std(fd.into_socket().into())
}
```
Because adopting the duplicated socket into compio can fail, `wrap_duped_fd`
should return `io::Result<TcpStream>` instead of a bare `TcpStream`.
The installer paths then need to handle that explicitly and preserve the
replica `on_done()` callback contract on failure.
### 3. `message_bus::installer`: handle `wrap_duped_fd` failures
Example shape:
```rust
match fd_transfer::wrap_duped_fd(fd) {
Ok(stream) => install_replica_inbound(self, stream, on_message, on_done),
Err(e) => {
warn!("failed to wrap delegated inbound replica fd: {e}");
on_done();
}
}
```
Client paths can log and drop the connection. Replica paths should call
`on_done()` so shard handoff accounting remains correct.
### 4. `message_bus::socket_opts`: avoid Unix `SOMAXCONN` and Windows
`SO_REUSEADDR`
`libc::SOMAXCONN` does not exist on Windows. A portable backlog constant
works:
```rust
const LISTEN_BACKLOG: i32 = 1024;
socket.listen(LISTEN_BACKLOG)?;
```
Also, Windows `SO_REUSEADDR` has different semantics than Unix and can allow
binding over a live listener. To preserve the current “fail if another process
is already listening” behavior, apply `set_reuse_address(true)` only on Unix:
```rust
#[cfg(unix)]
socket.set_reuse_address(true)?;
```
### 5. `message_bus::transports::tcp`: use portable socket shutdown
The TCP shutdown watchdog used raw Unix fd shutdown:
```rust
libc::shutdown(raw_fd, libc::SHUT_RD)
```
Since the watchdog holds `PollFd<socket2::Socket>`, this can use the
portable socket2 API instead:
```rust
use std::net::Shutdown;
if let Err(err) = poll_fd.shutdown(Shutdown::Read) {
debug!(%label, %peer, error = ?err, "tcp watchdog: read shutdown
returned");
}
```
This removes the Unix-only `AsRawFd`, `libc::shutdown`, and `SHUT_RD`
dependency from that path.
### 6. `server::segment_recovery`: replace Unix-only positioned I/O
`core/server/src/segment_recovery.rs` used Unix-only:
```rust
std::os::unix::fs::FileExt;
file.read_exact_at(...)
file.write_all_at(...)
```
Windows has equivalent primitives named `seek_read` and `seek_write`. A
small local extension trait can preserve the existing call sites:
```rust
trait PositionedFileExt {
fn read_exact_at(&self, buf: &mut [u8], offset: u64) -> io::Result<()>;
fn write_all_at(&self, buf: &[u8], offset: u64) -> io::Result<()>;
}
#[cfg(unix)]
fn read_at(file: &fs::File, buf: &mut [u8], offset: u64) ->
io::Result<usize> {
std::os::unix::fs::FileExt::read_at(file, buf, offset)
}
#[cfg(windows)]
fn read_at(file: &fs::File, buf: &mut [u8], offset: u64) ->
io::Result<usize> {
std::os::windows::fs::FileExt::seek_read(file, buf, offset)
}
```
Same idea for writes via Unix `write_at` vs Windows `seek_write`.
### 7. Directory fsync needs platform handling
Both `server::segment_recovery` and `journal::prepare_journal` fsync parent
directories after rename. That is valid on Unix, but Windows std/compio does
not expose a directory handle that can be opened and passed to `sync_all()` the
same way.
Suggested implementation keeps the Unix durability step and makes Windows a
no-op while preserving file-level syncs:
```rust
#[cfg(unix)]
fn fsync_dir(dir: &str) -> Result<(), ServerError> {
fs::File::open(dir)
.and_then(|handle| handle.sync_all())
.map_err(|source| {
error!(dir, error = %source, "failed to fsync a directory during
recovery");
ServerError::from(IggyError::CannotSyncFile)
})
}
#[cfg(windows)]
fn fsync_dir(_dir: &str) -> Result<(), ServerError> {
Ok(())
}
```
This is probably the main semantic tradeoff to be reviewed carefully:
Windows keeps file contents synced, but does not get the Unix-style
parent-directory fsync after rename.
### 8. Test-only platform gates
Two tests create self-referential symlinks to force `ELOOP`. On Windows,
creating symlinks may require elevated privileges or Developer Mode, so those
tests should stay Unix-only:
```rust
#[cfg(unix)]
#[compio::test]
async fn
given_unopenable_index_when_recovering_should_fail_stop_without_truncating() {
...
}
```
One `journal::file_storage` test used the shard executor with no blocking
pool. That test is Linux/io_uring-specific; on Windows, compio filesystem
operations need the normal runtime thread pool. I used a platform-specific test
runtime helper.
## Suggested PR scope
This can probably be split into:
1. Cross-platform socket/fd changes in `message_bus`
2. Cross-platform file/handle and positioned I/O changes in
`journal`/`server`
3. Windows build documentation for vcpkg `hwloc`/`pkgconf`
4. Optional CI follow-up: add `windows-latest` `cargo check -p server --bin
iggy-server` once the hwloc setup is automated
The core result is that these changes were enough for a native Windows/MSVC
`iggy-server.exe` release build to complete locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
