Wait for Actor Start message to be processed before returning workflo…

[Horneth]

…w ID

[mcovarr]

These caches represent sensitive internal WorkflowActor data that the sender() shouldn't have access to. It might be preferable to have the return type be Future[Unit] and return futureCaches map { _ => () } (or some other nicer way of throwing away the Future success value).

[Horneth]

Ah good catch !

[mcovarr]

👍 nice apart from the one gripe 😄

[mcovarr]

I don't think it should be part of this ticket, but if we wanted to support a higher request rate we could just generate a UUID and a persist a shell of a workflow execution without doing all the heavy lifting we do now before persisting.

[kshakir]

This passes the K.I.S.S. principle, but I personally hate the ask pattern, as it leads to timeouts and exceptions when the system gets busy. One should be theoretically be able to have the two actors to tell / receive messages instead, though I don't have the code ready at this second.

[Horneth]

@mcovarr I think we still need to ensure that the submission is correct before sending back a 201 with the workflow ID, which means being sure that everything necessary to start executing the workflow is ready (all DB executions succeeded etc...)
@kshakir I see your point, however in this case I don't think having the ask timing out is a problem, if a WorkflowActor takes forever to initialize itself then there is actually some bottleneck further down, and it might even be better to say "sorry but we're really too busy right now, retry later", than keeping waiting for WorkflowActors, which is going to trigger a timeout anyway since this comes from the "submit endpoint" and spray is not going to wait forever either.

[kshakir]

Discussed offline 👍 for this PR, with another to be added to prioritize if we want to remove the ask pattern.

[Horneth]

Created a tech debt ticket as per discussed with @kshakir and @mcovarr https://broadinstitute.atlassian.net/browse/DSDEEPB-2214