Pipelining commands on a Redis instance is deprecated and will be removed in Redis 5.0.0

[anton-co]

When using Redis version 4.6 I get this error:

Pipelining commands on a Redis instance is deprecated and will be removed in Redis 5.0.0.

redis.multi do
  redis.get("key")
end

should be replaced by

redis.multi do |pipeline|
  pipeline.get("key")
end

(called from /usr/local/bundle/gems/rpush-7.0.1/lib/rpush/client/redis/notification.rb:70:in `block in register_notification'}

[benlangfeld]

I'm planning to work on a fix here. I'm starting with rpush/modis#35 and rpush/modis#34. Once modis is entirely free of deprecation warnings I will address rpush.

[benlangfeld]

I think your issue was fixed by modis v4.0.1. Could you try again with that version, ensuring that you do not upgrade past redis v4.7.1?

[benlangfeld]

Modis v4.1.0 was released with fixes for several more deprecation warnings. An upgrade is recommended.

[rwojsznis]

Seems like it's still occurring on latest (4.2) modis (with latest redis 4.8.x) as well, but atm I can't put my finger on it 🤔 seem like Modis.with_connection just yields redis connection so from where this multi call is coming from? I guess it's still coming from modis somewhere? 🤔 Kinda giving up with this for now 😅

[bucknermr]

I'm also still seeing this issue on 4.2 and redis 4.8.1, but I think I've found the issue.

In Rpush::Client::Redis::Notification, there is the following after_create callback, which calls zadd directly on a Redis instance:

rpush/lib/rpush/client/redis/notification.rb

Line 9 in eb48a90

after_create :register_notification

rpush/lib/rpush/client/redis/notification.rb

Lines 68 to 72 in eb48a90

	def register_notification
	Modis.with_connection do |redis|
	redis.zadd(self.class.absolute_pending_namespace, id, id)
	end
	end

However, in Modis::Persistence, registered callbacks for saving a record are executed within a transaction (or #multi block): https://github.com/rpush/modis/blob/bfe2b160d59541223541e4195532de465513800e/lib/modis/persistence.rb#L212-L215
(Note that #destroy would theoretically have the same issue)

At a glance, it seems there are at least two ways forward:

1. Have Modis keep a reference to a current_transaction, and ensure that any calls to Redis use either the current_transaction, or if not in a transaction, then use a client from the pool.
2. Update Modis::Persistence so that callbacks are executed outside of the transaction.

Neither of these seem ideal, though someone with more context might understand better than I. For option 1, the caller would need to know that the callback is executed within a transaction and prevent calling certain methods. For option 2, you lose the atomicity of the after_create callback.

In any case, I tried option 2 above as a the simple fix and it did eliminate the warning.