feat(queue): add retryJobs method #1024

roggervalf · 2022-01-26T03:00:08Z

roggervalf · 2022-01-26T03:00:47Z

src/classes/queue.ts

@@ -361,6 +361,13 @@ export class Queue<
    } while (cursor);
  }

+  async retryAllFailedJobs(count?: number): Promise<void> {


probable we could rename this as retryFailedJobs, to be shorter 🤔

the minimal name would be just "retryFailed", or even just "retry" if in the future we would allow retry also "completed" jobs, which is not completely unlikely, the case being, a lot of jobs completed but they did not do the proper job, so we want to retry all of them.

maybe "retryJobs" then.

sounds good to me, I will change the count prop to be into an object, so when we introduce the case for completed jobs, a new param could be place into this object

manast · 2022-01-26T04:09:38Z

src/commands/retryAllFailedJobs-4.lua

+
+for i, key in ipairs(jobs) do
+  local jobKey = baseKey .. key
+  if (rcall("ZREM", KEYS[3], key) == 1) then


It should not be needed to check that we in fact removed the value from the zset since .lua scripts are atomic and we got the value from a previous call to ZRANGE. By skipping this test we can remove the values much faster in one call (up to 7000 values since there is an issue with more than 8k values in Redis), check how we did it here: https://github.com/taskforcesh/bullmq/blob/master/src/commands/moveStalledJobsToWait-8.lua#L103-L107
Same concept can be applied to LPUSH, however not possible with HDEL and HSET. I think the reset of finishOn, etc could be made when moving the job to active so that it is done lazily and therefore this call will become much faster, what do you think?
Also, regarding attemptsMade, I see there is a point in resetting it, but not sure we must, that would also make this call faster as we do not need to iterate through all the keys one by one.

Sadly we cannot optimize the call to XADD, so that will still need to be iterated...

manast · 2022-01-26T04:17:12Z

src/classes/queue.ts

@@ -361,6 +361,13 @@ export class Queue<
    } while (cursor);
  }

+  async retryAllFailedJobs(count?: number): Promise<void> {


the minimal name would be just "retryFailed", or even just "retry" if in the future we would allow retry also "completed" jobs, which is not completely unlikely, the case being, a lot of jobs completed but they did not do the proper job, so we want to retry all of them.

manast · 2022-01-26T04:17:32Z

src/classes/queue.ts

@@ -361,6 +361,13 @@ export class Queue<
    } while (cursor);
  }

+  async retryAllFailedJobs(count?: number): Promise<void> {


maybe "retryJobs" then.

manast · 2022-01-26T04:19:22Z

tests/test_queue.ts

+      });
+
+      fail = false;
+      await queue.retryAllFailedJobs(2);


It is also important to check that the order of the retried jobs is correct, as the oldest failed must be the first to be retried.

manast · 2022-01-26T04:20:03Z

Note: we need to backport this feature to Bull aswell.

manast · 2022-01-26T14:32:23Z

src/commands/retryJobs-4.lua

+  return 1
+end
+
+return 0


Notice that there is no newline at the end.

manast

Looks great, just a small comment regarding the documentation of the retryAll method.

manast · 2022-01-28T05:37:57Z

src/classes/queue.ts

@@ -361,6 +361,13 @@ export class Queue<
    } while (cursor);
  }

+  async retryJobs(opts: { count?: number } = {}): Promise<void> {


We should add a documentation snippet for this method so that it appears in the API documentation.

manast · 2022-01-28T05:39:12Z

src/commands/includes/batches.lua

@@ -0,0 +1,12 @@
+local function batches(n, batchSize)


Nice refactoring

manast · 2022-01-28T05:40:48Z

src/commands/retryJobs-4.lua

+  rcall("XADD", KEYS[2], "*", "event", "waiting", "jobId", key);
+end
+
+if (#jobs > 0) then


This "if" can also be moved to include also the "ipairs" loop

manast · 2022-01-28T05:41:04Z

src/commands/retryJobs-4.lua

+end
+
+if (#jobs > 0) then
+  for from, to in batches(#jobs, 7000) do


# [1.68.0](v1.67.3...v1.68.0) (2022-01-29) ### Features * **queue:** add retryJobs method for failed jobs ([#1024](#1024)) ([310a730](310a730)) ### Performance Improvements * **lua:** call del command with multiple keys ([#1035](#1035)) ([9cfaab8](9cfaab8))

github-actions · 2022-01-29T14:56:15Z

🎉 This PR is included in version 1.68.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

feat(queue): add retryAllFailedJobs method

e30e5a6

roggervalf commented Jan 26, 2022

View reviewed changes

roggervalf requested a review from manast January 26, 2022 03:01

manast requested changes Jan 26, 2022

View reviewed changes

roggervalf added 2 commits January 26, 2022 08:15

refactor: rename and change tests in retryJobs

14d1803

chore: merge branch master

8125d0c

roggervalf requested a review from manast January 26, 2022 14:28

manast reviewed Jan 26, 2022

View reviewed changes

src/commands/retryJobs-4.lua Outdated

return 1

end

return 0

Copy link

Contributor

manast Jan 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice that there is no newline at the end.

roggervalf added 2 commits January 26, 2022 18:12

style: add new line in script

b7c454b

chore: merge branch master

63e2601

roggervalf changed the title ~~feat(queue): add retryAllFailedJobs method~~ feat(queue): add retryJobs method Jan 27, 2022

roggervalf added 2 commits January 27, 2022 20:20

perf(retry-jobs): unpack jobs

c638429

chore: remove unused variables

9163b33

roggervalf requested a review from manast January 28, 2022 02:28

manast reviewed Jan 28, 2022

View reviewed changes

roggervalf added 4 commits January 28, 2022 09:59

refactor(retry-jobs): move ipairs logic into if block

60cdd8d

docs(retry-jobs): add description

f5d407f

chore: merge master

a810a12

chore: merge conflicts

3da3265

roggervalf requested a review from manast January 29, 2022 02:07

manast approved these changes Jan 29, 2022

View reviewed changes

roggervalf merged commit 310a730 into master Jan 29, 2022

roggervalf deleted the retry-all-failed-jobs branch January 29, 2022 14:52

github-actions bot added the released label Jan 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(queue): add retryJobs method #1024

feat(queue): add retryJobs method #1024

roggervalf commented Jan 26, 2022 •

edited

roggervalf Jan 26, 2022

manast Jan 26, 2022

manast Jan 26, 2022

roggervalf Jan 26, 2022

manast Jan 26, 2022

manast Jan 26, 2022

manast Jan 26, 2022

manast Jan 26, 2022

manast Jan 26, 2022

manast commented Jan 26, 2022

manast Jan 26, 2022

manast left a comment

manast Jan 28, 2022

manast Jan 28, 2022

manast Jan 28, 2022

manast Jan 28, 2022

github-actions bot commented Jan 29, 2022

feat(queue): add retryJobs method #1024

feat(queue): add retryJobs method #1024

Conversation

roggervalf commented Jan 26, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manast commented Jan 26, 2022

Choose a reason for hiding this comment

manast left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 29, 2022

roggervalf commented Jan 26, 2022 •

edited