vsivsi / meteor-job-collection

A persistent and reactive job queue for Meteor, supporting distributed workers that can run anywhere.
https://atmospherejs.com/vsivsi/job-collection
Other
385 stars 68 forks source link

problems with jc.removeJobs #173

Closed Nevtep closed 8 years ago

Nevtep commented 8 years ago

I'm trying to run a cleanup job to remove old jobs, code goes like this:

cleanupJobQueue = workers.processJobs('cleanup', function(job, cb) {
    console.log("Running cleanup job")
    var current, ids;
    current = new Date();
    current.setMinutes(current.getMinutes() - 60);
    console.log("cleanup - set current to:", current)
    ids = workers.find({
      status: {
        $in: Job.jobStatusRemovable
      },
      updated: {
        $lt: current
      }
    }, {
      fields: {
        _id: 1
      }
    }).map(function(d) {
      return d._id;
    });
    console.log("cleanup - found ids:", ids.length)

    workers.removeJobs(ids, {}, function(err, result) {
      if(!result)
        console.log("cleanup - error:", err);
      else
        console.log("cleanup - success");
      if (ids.length > 0) {
        console.log("Removed " + ids.length + " old jobs");
      } else {
        console.log("Nothing to remove");
      }
      job.done();
      cb();
    });
  });

The jobs are not being removed, this is the log I get:

Running cleanup job cleanup - set current to: Tue Apr 12 2016 04:48:48 GMT+0000 (UTC) cleanup - found ids: 8239

Nothing else beyond this line, no error, no exception thrown, no Nothing to remove, no messages at all and job is still "running on the db" (looks like the callback is bit being called) what could be causing this?

thank you for your answers

Nevtep commented 8 years ago

I've done some cleanup and refactoring, and found out that actually, only 256 jobs get removed, but still the callback was not called, I removed the callback and now I get this log:

pr 11 23:21:04 coinstructor app/web.1: Running cleanup job Apr 11 23:21:04 coinstructor app/web.1: cleanup - set current to: Tue Apr 12 2016 05:21:03 GMT+0000 (UTC) Apr 11 23:21:04 coinstructor app/web.1: cleanup - found ids: 7516 Apr 11 23:21:05 coinstructor app/web.1: cleanup - success Apr 11 23:21:05 coinstructor app/web.1: Removed 7516 old jobs Apr 11 23:21:19 coinstructor app/web.1: Running cleanup job Apr 11 23:21:19 coinstructor app/web.1: cleanup - found ids: 7261 Apr 11 23:21:19 coinstructor app/web.1: cleanup - set current to: Tue Apr 12 2016 05:21:18 GMT+0000 (UTC) Apr 11 23:21:22 coinstructor app/web.1: cleanup - success Apr 11 23:21:22 coinstructor app/web.1: Removed 7261 old jobs Apr 11 23:21:34 coinstructor app/web.1: Running cleanup job Apr 11 23:21:34 coinstructor app/web.1: cleanup - set current to: Tue Apr 12 2016 05:21:33 GMT+0000 (UTC) Apr 11 23:21:34 coinstructor app/web.1: cleanup - found ids: 7005 Apr 11 23:21:34 coinstructor app/web.1: cleanup - success Apr 11 23:21:34 coinstructor app/web.1: Removed 7005 old jobs Apr 11 23:21:49 coinstructor app/web.1: Running cleanup job Apr 11 23:21:49 coinstructor app/web.1: cleanup - found ids: 6749 Apr 11 23:21:49 coinstructor app/web.1: cleanup - set current to: Tue Apr 12 2016 05:21:48 GMT+0000 (UTC) Apr 11 23:21:50 coinstructor app/web.1: cleanup - success Apr 11 23:21:50 coinstructor app/web.1: Removed 6749 old jobs

code is like below:

  cleanupJobQueue = workers.processJobs('cleanup', function(job, cb) {
    console.log("Running cleanup job")
    var current, ids;
    current = new Date();
    current.setMinutes(current.getMinutes() - 60);
    console.log("cleanup - set current to:", current)
    ids = workers.find({
      status: {
        $in: Job.jobStatusRemovable
      },
      updated: {
        $lt: current
      }
    }, {
      fields: {
        _id: 1
      }
    }).map(function(d) {
      return d._id;
    });
    console.log("cleanup - found ids:", ids.length)

    if (ids.length > 0) {
      if(workers.removeJobs(ids)){
        console.log("cleanup - success");
        console.log("Removed " + ids.length + " old jobs");
      } else {
        console.log("cleanup - error");
      }
    } else {
      console.log("Nothing to remove");
    }

    job.done();
    cb();

  });

it's doing the job now, but I would like to see all the jobs gone, not only 256, how can I achieve that? thanks!

vsivsi commented 8 years ago

Doh, I just responded to this issue on the other project: https://github.com/vsivsi/meteor-job/issues/10

So it seems like my assumption that this was running in plain node.js was wrong. If this is running on a Meteor server, then authentication/permissions won't be an issue. 256 is a magic number here, because the underlying implementation "chunks" the delete requests into multiple 256 job requests to keep the DDP method calls reasonably small (DDP doesn't really like huge messages).

Anyway, if you could package up a simple reproduction as a github project, I'll look into this. It's possible that there's a meteor-job bug here, but I need a clear reproduction to proceed.

Thanks!

vsivsi commented 8 years ago

Just to be clear, a "reproduction" should be a full minimal Meteor app, so ideally I can just do "meteor run" in the project directory and see the problem reproduced. Thx.

vsivsi commented 8 years ago

I've confirmed that this was an (embarrassing) bug. Hinged on the difference between:

// Wrong!
retVal ||= methodCall(root, "jobRemove", [chunkOfIds, options], myCb);
// Right!
retVal = methodCall(root, "jobRemove", [chunkOfIds, options], myCb) || retVal;

Short circuit evaluation is occasionally pretty unintuitive...

This is fixed on the master branch. I'll release it to Atmosphere tomorrow morning.

Thanks for reporting this bug!

vsivsi commented 8 years ago

Fixed in 1.3.3, just published on Atmosphere / npm

vsivsi commented 8 years ago

Hi, Can you verify that 1.3.3 solves this issue for you?

vsivsi commented 8 years ago

No response for a week, so I assume this issue is resolved and I'm closing this one. If you have any additional problems with this issue, feel free to reopen it.