while_executing with on_conflict: :reschedule Reschedule job after job execution

[snyt45]

Describe the bug

I am not certain if this is a bug.
It started happening when I updated sidekiq unique-jobs from v8.0.2 to v8.0.3.

Environment in which it occurs：

• sidekiq v7.0.6
• sidekiq-unique-jobs v8.0.3

sidekiq-unique-jobs/lib/sidekiq_unique_jobs/lock/while_executing.rb

Lines 41 to 54 in f381562

	def execute(&block)
	with_logging_context do
	executed = locksmith.execute do
	yield
	ensure
	unlock_and_callback
	end
	unless executed
	reflect(:execution_failed, item)
	call_strategy(origin: :server, &block)
	end
	end
	end

Events:

1. execute a Job whose lock strategy is lock: :while_executing and whose conflict strategy is on_conflict: :reschedule.
2. the Job is executed, but the contents of executed is empty and enters the condition of unless executed and the Job is rescheduled.

The above events loop infinitely.

Expected behavior

In v8.0.2, the job_id is placed in executed and is not rescheduled.

v8.0.2

# lib/sidekiq_unique_jobs/lock/while_executing.rb
# Debugging code is embedded without changing the behavior of the original code itself.

      def execute(&block)
        with_logging_context do
          executed = locksmith.execute do
            yield
            if locksmith.unlock
              test_result1 = callback_safely
              puts "=== test_result1: #{test_result1}"
              test_result1
            end
          ensure
            test_result2 = locksmith.unlock
            puts "=== test_result2: #{test_result2}"
            test_result2
          end
          
          puts "=== executed: #{executed}"
          
          unless executed
            reflect(:execution_failed, item)
            call_strategy(origin: :server, &block)
          end
        end
      end

# 【result】
# === test_result1: f3a463a0f7d2256d93af271f
# === test_result2: false
# === executed: f3a463a0f7d2256d93af271f

Current behavior

In v8.0.3, the job is rescheduled because the executed is empty.

v8.0.3

# lib/sidekiq_unique_jobs/lock/while_executing.rb
# Debugging code is embedded without changing the behavior of the original code itself.

      def execute(&block)
        with_logging_context do
          executed = locksmith.execute do
            test_result1 = yield
            puts "=== test_result1: #{test_result1}"
            test_result1  
          ensure
            test_result2 = unlock_and_callback
            puts "=== test_result2: #{test_result2}"
            test_result2
          end

          puts "=== executed: #{executed}"

          unless executed
            reflect(:execution_failed, item)
            call_strategy(origin: :server, &block)
          end
        end
      end


# === test_result1:
# === test_result2: 8a68a05aa01571092bf97f4c
# === executed:

Additional context
Related issues：#770
Related PR：#771

[serggl]

Seeing the same on my end, had to downgrade to 8.0.2. The scheduled queue never gets clean

[pcothenet]

We saw the same problem, also downgraded to 8.0.2.

Not entirely sure if that's because we misunderstood that setting until now and the changes started making it work as expected, or if this is a bug.

[RobsonKing]

I have the same problem on v7.1.30 and it looks like the code in question was back-ported to V7.

[mhenrixon]

Reschedule isn't valid for while executing. If you want to retry it just use raise and let sidekiq handle when.

[snyt45]

@mhenrixon

Reschedule isn't valid for while executing. If you want to retry it just use raise and let sidekiq handle when.

Sorry, my explanation was insufficient.
The problem is not that it does not reschedule during execution, but that the executing is empty and rescheduled even though the job is running.
You probably expect the return value of the ensure block to be returned, but in fact the result of the yield is returned, so executing is always empty.

#770 (comment)

[mhenrixon]

Ok, I realesed v8.0.5 and v7.1.31 that should address this issue. Let me know if that's not the case.

[pcothenet]

Thank you! Seems to address the issue for us, appreciate the follow-up!