Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

About shutdown judgment of Worker Process #282

Closed
tmimura39 opened this issue Aug 19, 2024 · 9 comments
Closed

About shutdown judgment of Worker Process #282

tmimura39 opened this issue Aug 19, 2024 · 9 comments
Assignees
Milestone

Comments

@tmimura39
Copy link
Contributor

Hi, I am currently trying to get SolidQueue support for the job-iteration gem. Shopify/job-iteration#496

A brief description of job-iteration is as follows

Meet Iteration, an extension for ActiveJob that makes your jobs interruptible and resumable, saving all progress that the job has made (aka checkpoint for jobs).

iteration-how-it-works

Here is what needs to be addressed for each Adapter.

The job iterates over two records of the relation and then receives SIGTERM (graceful termination signal) caused by a deploy.
The signal handler sets a flag that makes job_should_exit? return true.

I want to determine if the SolidQueue Worker Process is about to receive a TERM and shutdown.

Is this judgment correct?

# frozen_string_literal: true

begin
  require "solid_queue"
rescue LoadError
  # SolidQueue is not available, no need to load the adapter
  return
end

module JobIteration
  module InterruptionAdapters
    module SolidQueueAdapter
      class << self
        attr_accessor :stopping

        def call
          stopping
        end
      end
    end

    ActiveSupport::Notifications.subscribe("graceful_termination.solid_queue") do
      JobIteration::Integrations::SolidQueueAdapter.stopping = true
    end

    register(:solid_queue, SolidQueueAdapter)
  end
end

GoodJob has implemented a new feature for this.
bensheldon/good_job#1253

I would appreciate your advice as I am unable to determine if the same action is required for SolidQueue.

@rosa
Copy link
Member

rosa commented Aug 20, 2024

Hey @tmimura39, thanks a lot for looking into this! 🙏 I think your judgement is correct, but it'd apply to all workers currently supervised by the supervisor that receives the TERM signal. I wonder if we'd rather have something that applies to a single worker, which will receive a TERM signal from the supervisor anyway 🤔 Then, the job could check that one. We should also have thread safety into account as in async mode.

I think it'd be nicer to have something like GoodJob, I'll look into it!

@rosa
Copy link
Member

rosa commented Aug 24, 2024

I had completely forgotten that @bdewater had commented about Shopify's job-iteration in #124 (comment), with some quite helpful insight:

It interacts in various ways with background queues: it uses the Sidekiq quiet callback, GoodJob recently added current_thread_shutting_down? to query for this, and for Resque a monkey patch is used.

IMO a callback is a bit more flexible since for other use cases one might not be able to/want to poll for this.

I'm going to look into providing a callback like Sidekiq as well.

@rosa rosa added this to the v1.0 milestone Aug 24, 2024
@rosa
Copy link
Member

rosa commented Sep 2, 2024

Thanks for your patience! I've added a couple of lifecycle hooks, start and stop, in #317. Do you think this could work to add an adapter that would be quite similar to the Sidekiq adapter?

@tmimura39
Copy link
Contributor Author

tmimura39 commented Sep 3, 2024

Thanks!

That is a very good mechanism, but it may not be available for job-iteration.
In job-iteration, we want to detect the receipt of a TERM signal in the process that actually processes the job (i.e., the Worker in SolidQueue).

Its lifecycle hooks (#317) are executed only in the Supervisor process.

When the Supervisor receives a TERM signal, it sends a TERM signal to the fork process including the Worker.

It would be nice to be able to set some kind of hook when this TERM signal is received in the Worker process.

I implemented and tested it this way.

module JobIteration
  module InterruptionAdapters
    module SolidQueueAdapter
      class << self
        attr_accessor :stopping

        # This is called by the Worker process.
        # Since the on_stop hook does not work within the Worker process, `sopping` is left unset.
        def call
          stopping
        end
      end
    end

    # on_stop hook works correctly. (in Supervisor Process)
    SolidQueue.on_stop do
      JobIteration::InterruptionAdapters::SolidQueueAdapter.stopping = true
    end

    register(:solid_queue, SolidQueueAdapter)
  end
end

On second thought, ActiveSupport::Notifications.subscribe(“graceful_termination.solid_queue”) was still insufficient.

The “graceful_termination.solid_queue” event is fired only in the Supervisor process, so it seems that the Worker process cannot detect it.

Please point out any errors in my perception.

@rosa
Copy link
Member

rosa commented Sep 3, 2024

Ah yes! You're right! I hadn't thought very well about this on the job-iteration gem; I was following the idea of subscribing to the graceful_termination.solid_queue event that as you said, is only fired in the supervisor.

I'll tweak this!

@rosa
Copy link
Member

rosa commented Sep 3, 2024

Ok, I've got on_worker_stop now, added in 18f89e5. I think this one should work!

@tmimura39
Copy link
Contributor Author

tmimura39 commented Sep 4, 2024

Excellent👍

Just what I wanted.
I was able to confirm that it works correctly in combination with job-iteration.

Thank you so much!

# frozen_string_literal: true

require "job-iteration"

module JobIteration
  module InterruptionAdapters
    module SolidQueueAdapter
      class << self
        attr_accessor :stopping

        def call
          stopping
        end
      end
    end

    SolidQueue.on_worker_stop do
      JobIteration::InterruptionAdapters::SolidQueueAdapter.stopping = true
    end

    register(:solid_queue, SolidQueueAdapter)
  end
end

@rosa
Copy link
Member

rosa commented Sep 4, 2024

Awesome! Thanks a lot. I'll merge this and release a new version with these changes 🙏

@rosa
Copy link
Member

rosa commented Sep 4, 2024

Done! Version 0.7.1 has these new hooks. Going to close this one then, but let me know if you encounter any problems 🙏 Thanks again for the patience and help!

@rosa rosa closed this as completed Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants