1

I have two methods:

def getNextJob: Future[Option[Job]]

def process(job: Job): Future[Unit]

I would like to process all Jobs until there are no jobs remaining.

I can do this with Await e.g.

private def process()(implicit ctx: ExecutionContext): Future[Unit] = {
    var job: Option[Job] = Await.result(service.getNextJob, FiniteDuration(2, TimeUnit.SECONDS))
    while(job.isDefined) {
      Await.result(process(job.get), FiniteDuration(2, TimeUnit.SECONDS))
      job = Await.result(service.getNextJob, FiniteDuration(2, TimeUnit.SECONDS))
    }
    Future.successful()
  }

But this is ugly and doesn't use Futures properly. Is there a way I could chain the futures somehow to replace this?

Eduardo
  • 6,900
  • 17
  • 77
  • 121
  • Do you want to `process` jobs in strict order as in your example code, or can they be executed in any order? – Tim Feb 11 '19 at 17:26
  • @Tim Any order is fine, I just need to ensure 1 runs at once – Eduardo Feb 11 '19 at 20:03
  • I guess I am confused about why `process` returns a `Future` if you always wait for it to complete before moving on to the next job. If you don't wait for it to complete then multiple jobs could be processed at the same time. – Tim Feb 12 '19 at 07:32
  • @Tim I wasn't going to go into the implementation but `getNextJob` just pulls one job that has status `unprocessed` from the database. `process` processes it and updates the status to `done`. If I processed multiple in parallel there is no guarantee (with my current implementation) that the job would only ever run once as two calls to `getNextJob` could potentially return the same job twice` – Eduardo Feb 12 '19 at 09:19
  • 1
    In that case you should probably avoid returning `Future` from `process` and just make it do the processing and then return `Unit`. This is already running in the `Future` from `getNextJob` so there is no need to have another `Future` nested inside the first `Future`. – Tim Feb 12 '19 at 09:44

2 Answers2

3
def go()(implicit ctx: ExecutionContext): Future[Unit] =
  getNextJob.flatMap { maybeJob ⇒
    if(maybeJob.isDefined) process(maybeJob.get).flatMap(_ ⇒ go())
    else Future.unit
  }

Note: It is not tail recursive.

Pritam Kadam
  • 2,388
  • 8
  • 16
  • 3
    It is actually not recursive at all :) Note, that `Option.get` is "code smell", and should be avoided. You should use something like `case Some(job) => process(job).flatMap(_ => go()) ; case None => Future.unit` instead – Dima Feb 11 '19 at 18:35
  • I went with the suggestions from @Dima for this answer, many thanks! – Eduardo Feb 12 '19 at 09:12
  • Note that since it's a future, the function not being tail recursive shouldn't be a problem https://stackoverflow.com/a/16986416/1507124 – CervEd Aug 09 '20 at 12:42
3
def processAll()(implicit ec: ExecutionContext): Future[Unit] =
  getNextJob.flatMap {
    case Some(job) => process(job).flatMap(_ => processAll())
    case None => Future.unit
  }

To process them all possibly concurrently:

def processAll()(implicit ec: ExecutionContext): Future[Unit] =
  getNextJob.flatMap {
    case Some(job) => process(job).zipWith(processAll())((_,_) => ())
    case None => Future.unit
  }
Viktor Klang
  • 26,479
  • 7
  • 51
  • 68
  • this will process the jobs one at a time, correct? If so, how to modify it to process the jobs concurrently? – CervEd Aug 09 '20 at 13:49
  • 1
    @CervEd Added that. Warning though, I wrote it from memory so it might not compile – Viktor Klang Sep 06 '20 at 12:55
  • Thank you! I used something like `Future.sequence { (1 to X).map(processAll(...)) }`, where X is how many requests I wanted processed at a time, but it felt incorrect – CervEd Sep 07 '20 at 10:42