deadpool-lapin Manages connections but shouldn't it be Pooling Channels instead?

[Overdash]

Hey,

I've been doing some research on optimising RabbitMQ Connections and Channels. I found that Connections should NOT be pooled. Instead Channels should be reused. Although, in terms of benchmarks, there's very little difference. between Pooling Connections vs Channels, it seems for the former is an Anti-pattern since connections are supposed to be long-lives and using a lot of connections drops performance. Not sure if anyone else has looked into this?

I created a Channel Manager (for deadpool 0.7.0, v0.8 breaks this code).

// ... lapin imports stub
pub type ChannelPool = deadpool::managed::Pool<Channel, anyhow::Error>;
pub type ManagedChannel = deadpool::managed::Object<Channel, anyhow::Error>;

type RecycleResult = deadpool::managed::RecycleResult<anyhow::Error>;
type RecycleError = deadpool::managed::RecycleError<anyhow::Error>;

pub struct ChannelManager {
    conn: Connection,
    rt: Option<Runtime>
}

impl ChannelManager {
    pub fn new(conn: Connection) -> Self {
        Self {
            conn,
            rt: None
        }
    }

    pub fn add_runtime(&mut self, rt: Runtime) {
        // check a runtime exists; only add if one doesn't; we should not swap out runtimes
        match self.rt {
            None => self.rt = Some(rt),
            _ => warn!("Runtime already exists for this manager. Cannot swap out existing runtimes")
        };
    }
}

#[async_trait]
impl deadpool::managed::Manager<Channel, Error> for ChannelManager {
    async fn create(&self) -> Result<Channel, Error> {
        let channel = self.conn.create_channel().await?;
        set_prefetch(&channel, 1).await?;
        Ok(channel)
    }

    async fn recycle(&self, channel: &mut Channel) -> RecycleResult {
        match channel.status().state() {
            lapin::ChannelState::Connected => Ok(()),
            other_state => Err(RecycleError::Message(format!(
                "MQ Channel #{} is in state: {:?}",
                channel.id(),
                other_state
            )))
        }
    }
}

[bikeshedder]

The reason why I haven't given that a try, yet is because I didn't see a way to properly recycle a channel.

Another issue I see here is what about disconnects? Does the lapin connection object implement some automatic reconnect logic that I'm not aware of?

Apart from those two issues it's cool and I'm more than happy to add this to deadpool-lapin. 👍

[Overdash]

I'll look into the recycling but I don't think lapin connection has auto-reconnect