film42 / sidekiq-rs

A port of sidekiq to rust using tokio
MIT License
95 stars 10 forks source link

Support redis namespaces #8

Closed film42 closed 1 year ago

film42 commented 1 year ago

Impl https://github.com/film42/sidekiq-rs/issues/7

This adds a shim around the redis connection to support namespaces similar to the https://github.com/resque/redis-namespace gem. This allows cooperation with ruby apps that depend on this feature.

There may be a slight uptick in allocations because I can't be as clever about passing by reference since we're going to mutate all keys for each redis call but that's something we may be able to optimize later.

The test suite is passing once again and I added an example showing how to activate the namespace mode.

Going to let this sit for a day and come back to review.

@justmark Would you mind trying to run your project against this PR?

To use namespace:

    // Redis
    let manager = RedisConnectionManager::new("redis://127.0.0.1/")?;
    let redis = Pool::builder()
        .connection_customizer(sidekiq::with_custom_namespace("cool_app".to_string())) // Do this!
        .build(manager)
        .await?;

NOTE: This does introduce a breaking change. We switch from Pool<RedisManagedConnection> to the sidekiq::RedisPool type and drop the bb8_redis dep. Instead, we implement our own redis pool.

justmark commented 1 year ago

@film42 I'll try this out early this afternoon

justmark commented 1 year ago

@film42 Nice, that worked. The speed I was getting was great, too. I had been doing some testing of using RabbitMQ, and your library with Sidekiq blows it out of the water as far as performance is concerned. Note that I had upgraded to bb8 0.8.0, and that does throw an error:

the trait bound `RedisConnectionManager: ManageConnection` is not satisfied
the trait `ManageConnection` is not implemented for `RedisConnectionManager`

bb8 0.7.0 works fine though.

justmark commented 1 year ago

Hi,

My worker needs access to MongoDB. I was looking at your register function to see if there was a way to pass a database into the worker, but I'm not seeing anything that does this. Do you have any suggestions?

Mark

film42 commented 1 year ago

There's a new bb8 out with a subtle change and I need to upgrade this lib. I'll do that next. Pretty easy upgrade.

If your worker needs access to mongo, you can pass a connection to the instantiation of your worker.

#[derive(Clone)]
struct SomeWorker {
  mongo: MongodbConnectionPoolFromSomeLib 
}

async fn main() {
  // ...
  p.register(SomeWorker{ mongo: my_mongo_conn.clone() })
}

Which will be available in your perform function. So long as the argument is cloneable you can access it from the worker.

justmark commented 1 year ago

Hi,

That worked like a charm. My workers also need to be able to access Redis directly. In addition to passing in the Mongo.clone as per your suggestion, I also did the same to pass in a Redis multiplexed connection. This is working, but I was wondering if there was a different way you'd suggest accessing Redis considering there is already a Redis connection supporting Sidekiq.

Thanks!

film42 commented 1 year ago

I would do the same thing as you outlined. You can pass in the RedisPool to your struct if you'd like. The redis pool can access the underlying (unnamespaced) redis connection by doing the following:

let x: String: pool.get().await?.unnamespaced_borrow_mut().get("x").await?;

Where unnamespaced_borrow_mut will give you a mutable ref to the inner redis::Connection connection type.