pingora icon indicating copy to clipboard operation
pingora copied to clipboard

Not possible to implement `pingora::services::Service` due to `Fds` not being available.

Open halzy opened this issue 1 year ago • 5 comments

Describe the bug

When trying to implement pingora::services::Service the Fds struct is not public because the transfer_fd module is private: error[E0603]: module `transfer_fd` is private

Pingora info

Pingora version: 0.1.0 Rust version: cargo 1.76.0 (c84b36747 2024-01-18) Operating system version: OSX 14.2.1 (23C71)

Steps to reproduce

When trying to implement the pingora::services::Service trait it is not possible due to the 2nd argument of start_service - fds type not being pub:

struct MyService {}

#[async_trait]
impl pingora::services::Service for MyService {
    async fn start_service(
        &mut self,
        fds: Option<Arc<tokio::sync::Mutex<pingora::server::transfer_fd::Fds>>>,
        mut shutdown: pingora::server::ShutdownWatch,
    ) {
        todo!()
    }

    fn name(&self) -> &str {
        todo!()
    }
}

Expected results

The Fds or ListenFds type is public.

In pingora-core/src/server/mod.rs ListenFds is pub(crate): 52:pub(crate) type ListenFds = Arc<Mutex<Fds>>;

in pingora-core/src/server/mod.rs the transfer_fd mod is pub(crate): 19:pub(crate) mod transfer_fd;

Observed results

What actually happened?

error[E0603]: module `transfer_fd` is private

halzy avatar Mar 20 '24 04:03 halzy

If one tries to copy the Fds struct up into their app to implement it the error is:

error[E0053]: method `start_service` has an incompatible type for trait

The struct copied is:

pub struct Fds {
    map: HashMap<String, std::os::fd::RawFd>,
}

halzy avatar Mar 20 '24 14:03 halzy

Thanks @halzy I've patched this in #149

bsodmike avatar Mar 22 '24 13:03 bsodmike

Thanks @halzy I've patched this in #149

I see. I'm not certain that the other public functions in that module are useful in the public interface as they are part of the machinery for transfering the sockets between pingoras. 🤷 We'll see what the folks at Cloudflare want to do.

halzy avatar Mar 22 '24 15:03 halzy

Do you have any sample code that actually uses the contents of Option<Arc<tokio::sync::Mutex<pingora::server::transfer_fd::Fds>>>. That would help me provide a better answer.

bsodmike avatar Mar 22 '24 15:03 bsodmike

Nothing that I can share at the moment. I posted the patch that solved my issue in #146.

halzy avatar Mar 22 '24 15:03 halzy