brandonhilkert / sucker_punch

Sucker Punch is a Ruby asynchronous processing library using concurrent-ruby, heavily influenced by Sidekiq and girl_friday.
MIT License
2.65k stars 114 forks source link

Passing non-primitives as arguments to perform? #136

Closed mraaroncruz closed 8 years ago

mraaroncruz commented 8 years ago

With other popular ruby job queues, it is recommended to only pass primitives as arguments to the jobs. I believe this is because the args are stored in a hash in Redis and not straight up marshaled.

Since there is no backing store, do we have this limitation in sucker_punch, or can I just send in, for example, an ActiveRecord model as an argument to perform?

Thanks!

mraaroncruz commented 8 years ago

Relevant sidekiq link

brandonhilkert commented 8 years ago

Passing in non-primitives should work fine. However, if there's significant back pressure, this could be a source of significant memory bloat with the pool thread.

mraaroncruz commented 8 years ago

Because waiting jobs would hold the whole object in memory. I didn't think of that — a really good argument against.

jdantonio commented 8 years ago

If you are using sucker_punch with Rails ActiveJob then you are limited to simple types, as documented in Rails. All the queue adapters serialize the entire Job object using a common serialization method. Passing arguments of unsupported types will lead to undefined behavior.

If you are not using ActiveJob and you choose to pass complex types, it's critical that you release the reference to the object (let any variable referencing the object go out of scope or reassign them). Retaining a reference to an object passed as a parameter may lead to concurrency errors (shared, mutable, state). I suspect this is the reason ActiveJob serializes the job--it guarantees that not references are shared.

mraaroncruz commented 8 years ago

For my use case, the inputs aren't being mutated. I just don't want to have to persist them before I transform a copy of them. It seems silly to have to persist, then pull that out and destroy it immediately after. If I was doing something more distributed, I would use something a little more heavy duty than sucker_punch and be forced to use primitives.

But if memory becomes an issue, it will would make sense to just pass an id or something.