[Backgroundrb-devel] Bug report: delete_worker breaks begin...ensure (was Re: Canceling work...)

[hemant]

On Tue, Feb 26, 2008 at 1:06 AM, John Wells <john.wells at greatworx.com> wrote:
>
> On Sun, Feb 24, 2008 at 1:37 PM, hemant <gethemant at gmail.com> wrote:
>  > Hi,
>  >
>  >
>  >
>  >  On Sun, Feb 24, 2008 at 6:51 AM, Yves-Eric Martin
>  >  <yem_backgroundrb-devel at filter.yve.net> wrote:
>  >  > Hi all, and welcome back John.
>  >  >
>  >  >
>  >  >  John, you may have missed my second reply to you:
>  >  >  http://rubyforge.org/pipermail/backgroundrb-devel/2008-February/001514.html
>  >  >
>  >  >  Anyway, the bad news is: I just tried it out and it does not seem to
>  >  >  work. delete_worker *is* the "kill -9" hemant suggested.
>  >  >
>  >  >  Hemant, this is breaking some basic Ruby functionality, namely
>  >  >  "begin...ensure". So can we please call this a bug? delete_worker should
>  >  >  be a "kill", not a "kill -9", or at least not by default. We could add
>  >  >  an argument to determine which behavior we want:
>  >  >
>  >  >  # normal "kill" behavior:
>  >  >  MiddleMan.delete_worker(:worker => :background_worker)
>  >  >
>  >  >  # brutal "kill -9" behavior:
>  >  >  MiddleMan.delete_worker(:worker => :background_worker, :kill => true)
>  >  >
>  >  >
>  >  >
>  >  >  Here is first some code I used in the console to demonstrate in plain
>  >  >  Ruby what should happen:
>  >  >
>  >  >  ##########################################################
>  >  >   def do_work(args=nil)
>  >  >     job_exit_status = 'FORCED_EXIT'
>  >  >     puts "#{Time.now} -- do_work entered"
>  >  >     begin
>  >  >       # Large batch job here
>  >  >       sleep 30
>  >  >     rescue
>  >  >       job_exit_status = 'ERRORED_OUT'
>  >  >       # Error processing here
>  >  >     else
>  >  >       job_exit_status = 'FINISHED_NORMALLY'
>  >  >     ensure
>  >  >       puts "#{Time.now} -- do_work exit status: #{job_exit_status}"
>  >  >     end
>  >  >   end
>  >  >  ##########################################################
>  >  >
>  >  >
>  >  >  If Ctrl-C is pressed before completion, here is what we get:
>  >  >
>  >  >   >> do_work
>  >  >  2008-02-24 10:12:03 -- do_work entered
>  >  >  2008-02-24 10:12:04 -- do_work exit status: FORCED_EXIT
>  >  >
>  >  >
>  >  >
>  >  >  Now here is the BackgrounDRb code used to show the bug:
>  >  >
>  >  >  ##########################################################
>  >  >  class BackgroundWorker < BackgrounDRb::MetaWorker
>  >  >   set_worker_name :background_worker
>  >  >   def create(args = nil); end
>  >  >   def do_work(args=nil)
>  >  >     job_exit_status = 'FORCEABLY_DELETED'
>  >  >     logger.info "#{Time.now} -- do_work entered"
>  >  >     begin
>  >  >       # Large batch job here
>  >  >       sleep 30
>  >  >     rescue
>  >  >       job_exit_status = 'ERRORED_OUT'
>  >  >       # Error processing here
>  >  >     else
>  >  >       job_exit_status = 'FINISHED_NORMALLY'
>  >  >     ensure
>  >  >       logger.info "#{Time.now} -- do_work exit status: #{job_exit_status}"
>  >  >     end
>  >  >   end
>  >  >  end
>  >  >  ##########################################################
>  >  >
>  >  >  If allowed to finish, we get this in the logs:
>  >  >  2008-02-24 09:45:37 -- do_work entered
>  >  >  2008-02-24 09:45:47 -- do_work exit status: FINISHED_NORMALLY
>  >  >
>  >  >  However, if delete_worker is called before completion, we only get:
>  >  >  2008-02-24 09:46:03 -- do_work entered
>  >
>  >  Eric,
>  >
>  >  Problem is if you look into code of delete_worker then, you will find
>  >  that we are sending TERM and KILL signals to the worker and hence its
>  >  not possible to use "ensure" there.
>  >
>  >  Why, we have both "TERM" and "KILL" there is because, signals to kill
>  >  a task is not cross platform across unixes.
>  >  But if we remove "KILL" and have "TERM" only, your worker will work as
>  >  you expected.
>  >
>  >  Along with Alex patches, I am trying to push this change in git, so
>  >  you can try. But, currently gitorious is down, sadly. I am literally
>  >  waiting for getting the damn thing up.
>
>  Thanks guys. I'm looking forward to seeing these patches.

They are there in the git repo, you can get latest code using:

git clone git://gitorious.org/backgroundrb/mainline.git backgroundrb

Also, you need to do:

sudo gem install packet chronic