Repository navigation
Port Hyperdrive to Rust, behind an autogate - #8146
bartlomieju wants to merge 3 commits into
Conversation
Hyperdrive::connect() builds its Socket from the binding's subrequest channel, host and port only, so the work moves to a free function, connectHyperdrive(js, clientIndex, host, port), and connectToDb() to a helper in hyperdrive.c++. Hyperdrive::connect() calls it, and so do the connect overrides, through it. This lets another implementation of the binding connect the same way. No change in behaviour: the Socket's address and domain are still the binding's random host and port.
The Rust Hyperdrive in src/workerd/api/hyperdrive is created for a Hyperdrive binding when the RUST_HYPERDRIVE autogate is on. It is the first production consumer of jsg-cpp (#8086): connect() returns the C++ Socket as a jsg_cpp::Ref<Socket>. - lib.rs: the resource. The same seven lazy, read-only instance properties as C++ and connect(). The connect and DNS overrides are registered on first use of the host, the IP, the connection string or connect(), as in C++. Finding a free synthetic address in 240.0.0.0/16 is plain Rust now, with the same walk from a random start and the same error. - ffi.rs and ffi.{h,c++}: the per-type shims of Socket, and per-method shims, run in jsg_cpp::callCpp(), over connectHyperdrive(), the global scope's connect and DNS overrides, and getEntropy(). hyperdrive_new() is the constructor C++ calls. - workerd-api.c++ creates the Rust Hyperdrive when the autogate is on. The C++ class stays registered, so the generated types are unchanged. Rust JSG has no JSG_MEMORY_INFO yet, so the Rust Hyperdrive reports nothing to heap snapshots, where the C++ one reports its strings. The Socket shims sit with Hyperdrive until a second Rust API needs them. Workers' API surface is unchanged.
testHyperdriveParity() runs one script, with the RUST_HYPERDRIVE autogate off ("Server:
Hyperdrive in C++") and on ("Server: Hyperdrive in Rust"), and expects the same output from
both. For a Postgres and a MySQL binding it checks:
- Object.keys order, the property descriptors and the values;
- the host and synthetic IP formats, that a read returns the same value, and that the two
bindings get different ones;
- the connection string;
- the prototype's members, the constructor, connect's name and length;
- Object.prototype.toString, the Illegal invocation error and assigning to a read-only property;
- that connect() returns a Socket, and that cloudflare:sockets connect() to each binding's host
and IP reaches that binding's upstream.
Under @all-autogates both cases run the Rust Hyperdrive, as the existing Hyperdrive server tests
then do.
guybedford
left a comment
There was a problem hiding this comment.
Nice first consumer of jsg-cpp, and the parity test (both runs checked against one expected output) is a good pattern. I ran hyperdrive_test, server-test@ and server-test@all-autogates locally and they pass. I also confirmed new proto.constructor() throws Illegal constructor in both implementations, if you want to add that case.
On type drift: the C++ class stays the source of the generated types while the Rust one is what runs. The parity test pins the runtime shape, but not things that exist only in the types, like argument and return types. Once #8077 lands, could a follow-up add a test that the RTTI exported for the Rust Hyperdrive equals the C++ one's? That stops the types drifting while both exist, and lets the Rust one take over as the source when the C++ class goes, with no snapshot change.
AI generated under my review
| case Global::HYPERDRIVE: { | ||
| auto hyperdrive = global.getHyperdrive(); | ||
| if (util::Autogate::isEnabled(util::AutogateKey::RUST_HYPERDRIVE)) { | ||
| auto str = [](capnp::Text::Reader text) { return ::rust::Str(text.begin(), text.size()); }; |
There was a problem hiding this comment.
::rust::Str(ptr, len) checks for valid UTF-8 and throws std::invalid_argument if it isn't. capnp Text isn't validated, so a config with non-UTF-8 bytes in the user or password fails here, while the C++ path accepts it. It's an edge case, but it's a parity gap. Perhaps pass &[u8] / rust::Slice<const uint8_t> across and decide in Rust (lossy, or a proper error)?
| let port = self.get_port(); | ||
| let mut random = [0_u8; 2]; | ||
| ffi::fill_entropy(&mut random)?; | ||
| let ip = find_free_ip(u16::from_be_bytes(random), |candidate| { |
There was a problem hiding this comment.
find_free_ip calls has_connect_override through call_cpp once per address it tries, up to 65536 times, each with a TryCatch and two strings. Usually it's one call, so this is a nit. A single shim that does the walk and the registration in C++ and returns the IP would cross once, and make check-then-register one step. Fine to keep the walk in Rust if that's the goal.
Ports the
Hyperdrivebinding to Rust JSG, behind a newRUST_HYPERDRIVEautogate. This is thefirst production consumer of
jsg-cpp(#8086):connect()returns the C++Socketas ajsg_cpp::Ref<Socket>.Important
Prerequisite: #8145 (Rust JSG heap snapshot memory info). Until it lands, the Rust
Hyperdrivereports nothing to heap snapshots, where the C++ one reports its strings.
Commits
connectHyperdrive(). The connection is a free function,connectHyperdrive(js, clientIndex, host, port), thatHyperdrive::connect()and the connectoverrides call. No change in behaviour.
src/workerd/api/hyperdrive/lib.rs: the resource, with the same seven lazy, read-onlyinstance properties as C++ (
database,user,password,host,ip,port,connectionString) andconnect().ffi.rs/ffi.{h,c++}: the per-type shims ofSocket, and per-method shims run injsg_cpp::callCpp()overconnectHyperdrive(), the global scope's connect and DNS overrides,and
getEntropy().hyperdrive_new()is the constructor C++ calls.workerd-api.c++creates the RustHyperdrivefor a Hyperdrive binding when the autogate ison.
server-test.c++.Behaviour
The Rust implementation is meant to be indistinguishable from the C++ one:
host,ip,connectionStringorconnect(), as in C++.getEntropy(), as in C++.240.0.0.0/16from a random start,failing with the same error once every address is taken.
connectHyperdrive(), so the I/O path, the Socket'saddress and domain, and the connection errors are shared.
Hyperdrivestays registered with the isolate, so the generated types are unchangedand nothing here depends on Generate Typescript types for Rust JSG Bindings #8077.
Workers' API surface is unchanged.
Known gaps
Socketper-type shims are in Hyperdrive'sffi.c++. They should move next tosockets.honce a second Rust API needs
Socket.ffi.c++binds C++ classes, so likejsg-test/cpp-interop.c++it uses C++ JSG and is not inCPP_SHIMSof the cpp-jsg allowlist test.