Skip to content

Commit

Permalink
Deprecate calling commands on the original Redis instance in multi {}
Browse files Browse the repository at this point in the history
The new favored API is:

```ruby
redis.multi do |transaction|
  transaction.get("foo")
  transaction.del("bar")
end
```

This API allow multiple threads to build pipelines concurrently on the same
connection, and is more friendly to Fiber based concurrency.

Fix: redis#1057
  • Loading branch information
byroot committed Jan 21, 2022
1 parent 9745e22 commit 025bb2a
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 13 deletions.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@
pipeline.get("key")
end
```
* Deprecate calling commands on `Redis` inside `Redis#multi`. See #1059.
```ruby
redis.multi do
redis.get("key")
end
```

should be replaced by:

```ruby
redis.multi do |transaction|
transaction.get("key")
end
```
* Deprecate `Redis#queue` and `Redis#commit`. See #1059.

* Fix `zpopmax` and `zpopmin` when called inside a pipeline. See #1055.
Expand Down
26 changes: 17 additions & 9 deletions lib/redis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def unwatch
def pipelined(&block)
deprecation_displayed = false
if block&.arity == 0
Pipeline.deprecation_warning(Kernel.caller_locations(1, 5))
Pipeline.deprecation_warning("pipelined", Kernel.caller_locations(1, 5))
deprecation_displayed = true
end

Expand Down Expand Up @@ -263,19 +263,27 @@ def pipelined(&block)
#
# @see #watch
# @see #unwatch
def multi
synchronize do |prior_client|
if !block_given?
prior_client.call([:multi])
else
def multi(&block)
if block_given?
deprecation_displayed = false
if block&.arity == 0
Pipeline.deprecation_warning("pipelined", Kernel.caller_locations(1, 5))
deprecation_displayed = true
end

synchronize do |prior_client|
begin
@client = Pipeline::Multi.new(prior_client)
yield(self)
prior_client.call_pipeline(@client)
pipeline = Pipeline::Multi.new(prior_client)
@client = deprecation_displayed ? pipeline : DeprecatedPipeline::Multi.new(pipeline)
pipelined_connection = PipelinedConnection.new(pipeline)
yield pipelined_connection
prior_client.call_pipeline(pipeline)
ensure
@client = prior_client
end
end
else
send_command([:multi])
end
end

Expand Down
23 changes: 19 additions & 4 deletions lib/redis/pipeline.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,19 @@ class Pipeline
STDLIB_PATH = File.expand_path("..", MonitorMixin.instance_method(:synchronize).source_location.first).freeze

class << self
def deprecation_warning(caller_locations) # :nodoc:
def deprecation_warning(method, caller_locations) # :nodoc:
callsite = caller_locations.find { |l| !l.path.start_with?(REDIS_INTERNAL_PATH, STDLIB_PATH) }
callsite ||= caller_locations.last # The caller_locations should be large enough, but just in case.
::Redis.deprecate! <<~MESSAGE
Pipelining commands on a Redis instance is deprecated and will be removed in Redis 5.0.0.
redis.pipelined do
redis.#{method} do
redis.get("key")
end
should be replaced by
redis.pipelined do |pipeline|
redis.#{method} do |pipeline|
pipeline.get("key")
end
Expand Down Expand Up @@ -195,7 +195,22 @@ def initialize(pipeline)

def __getobj__
unless @deprecation_displayed
Pipeline.deprecation_warning(Kernel.caller_locations(1, 10))
Pipeline.deprecation_warning("pipelined", Kernel.caller_locations(1, 10))
@deprecation_displayed = true
end
@delegate_dc_obj
end
end

DeprecatedPipeline::Multi = DelegateClass(Pipeline::Multi) do
def initialize(pipeline)
super(pipeline)
@deprecation_displayed = false
end

def __getobj__
unless @deprecation_displayed
Pipeline.deprecation_warning("multi", Kernel.caller_locations(1, 10))
@deprecation_displayed = true
end
@delegate_dc_obj
Expand Down

0 comments on commit 025bb2a

Please sign in to comment.