cloudflare / pingora

A library for building fast, reliable and evolvable network services.
Apache License 2.0
20.25k stars 1.1k forks source link

rwlock is locked in lb #223

Closed hubertshelley closed 1 month ago

hubertshelley commented 2 months ago

What is the problem your feature solves, or the need it fulfills?

 Rwlock is locked in lb, and I not sure it's a bug or designed. 

Describe the solution you'd like

 Rwlock is free in request_filter. 

Additional context



pub(crate) struct LB {
    pub(crate) service_book: Arc<RwLock<ServiceBook>>,
}

#[async_trait]
impl ProxyHttp for LB {
    type CTX = LBContext;

    fn new_ctx(&self) -> Self::CTX {
        Default::default()
    }

    ...

    async fn request_filter(
        &self,
        session: &mut Session,
        ctx: &mut Self::CTX,
    ) -> pingora::Result<bool>
    where
        Self::CTX: Send + Sync,
    {
        let path = session.req_header().uri.path().to_string();
        // self.service_book.read().await is locked
        match self.service_book.read().await.choose_service(path) {
            None => {
                println!("service not found");
                return Ok(false);
            }
            Some(service) => {
                service.url_rewrite(session.req_header_mut());
                ctx.service_addr = Some(service.get_url());
                Ok(true)
            }
        }
    }
}



vicanso commented 2 months ago

Is it really necessary to use rwlock? My project supports find a HttpPeer and rewrite the request path without lock.

https://github.com/vicanso/pingap/blob/main/src/proxy/server.rs#L375

eaufavor commented 2 months ago

I'm not sure if this is relevant to the Pingora framework. This is a user defined lock. Only you can lock it. Where is write() called for the lock?

github-actions[bot] commented 1 month ago

This question has been stale for a week. It will be closed in an additional day if not updated.

github-actions[bot] commented 1 month ago

This issue has been closed because it has been stalled with no activity.