I think the hook is needed because I took too much time to figure out the problem and much more time to figure out the solution (this master_run variable). Also, I don't think this master_run solution is elegant.
cheers, bráulio On Fri, Oct 3, 2014 at 10:22 PM, Eric Wong <[email protected]> wrote: > Bráulio Bhavamitra <[email protected]> wrote: >> The problem is actually worser, and the worker.nr == 0 can't be used. >> I had to do something like this: >> >> master_run = true >> >> before_fork do |server, worker| >> if master_run >> #warm up server... >> >> #kill old pid... >> >> #disconnect active record >> >> master_run = false >> end >> >> # other stuff for each worker >> end >> >> In the last example, using worker.nr == 0 would make the server crash >> in case the worker 0 was killed/restarted. >> >> Also the AR's disconnect and *many other stuff* people put on >> before_fork should only be run once the master was preloaded, not for >> every worker. > > AR disconnect! is idempotent and has been since unicorn existed. > I suspect most other things people run in before_fork are already > idempotent, too. > >> So I still think at least a hook like master_preloaded(server) is necessary. > > Using the local 'master_run' variable in your before_fork already > accomplishes everything you needed, right? > > I try to keep unicorn as lean as possible and not bloat it with > redundant features. The current hooks already fulfill the minimal > requirements for supporting apps which do not behave properly under > preload_app=true, so I am not convinced why a new hook is necessary.
