Allow up to max_inflight many concurrent function invocations. #95
Conversation
|
Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide. |
Fixes openfaas#94 Switch from auto ack to manual ack. This enables message handling in individual go routines. In main.go switch to atomic counter and include index in traces. Going forward these may be mixed with concurrent traces. Removes option to run without durable. Generates durable name base on subject. Removes issue where the durable would not see every messages. Signed-off-by: Matthias Hanel <mh@synadia.com>
matthiashanel
force-pushed
the
concurrent
branch
from
0c5e4b3
to
911f246
Compare
|
@LucasRoesler PTAL |
| @@ -34,12 +35,6 @@ func main() { | |||
|
|
|||
| hostname, _ := os.Hostname() | |||
|
|
|||
| var durable string | |||
| if config.NatsDurableQueueSubscription { | |||
There was a problem hiding this comment.
Is any helm chart update required, or the project README?
|
Thank you for the PR and for the attention to detail, we had several things "wrong" here and it's great to have the NATS team help us out. What are your thoughts on extracting the closure for processing messages to its own function? |
|
@matthiashanel please could you make an update to the README.md for the changes made in this PR? A quick update in the docs for the async page would also be appreciated, if you have time? https://github.com/openfaas/docs https://github.com/openfaas/docs/blob/master/docs/reference/async.md |
|
I think this line in the README needs to be removed after this PR, since the durable config was also removed in this PR https://github.com/openfaas/nats-queue-worker/blame/master/README.md#L37 |
|
Agreed, I also mentioned that here -> #95 (comment) @matthiashanel is in the US so will be online a little later than us in Europe. The new options also need to be documented here and in the docs website. |
|
@alexellis @LucasRoesler hey, I will update the readme in the same change I'll also account for the helm change for max in flight |
documenting max_inflight which was changed in: openfaas/nats-queue-worker#95) Fixes openfaas#225 Signed-off-by: Matthias Hanel <mh@synadia.com>
documenting max_inflight which was changed in: openfaas/nats-queue-worker#95) Fixes #225 Signed-off-by: Matthias Hanel <mh@synadia.com>
Description
Fixes #94
Switch from auto ack to manual ack.
This enables message handling in individual go routines.
In main.go switch to atomic counter and include index in traces.
Going forward these may be mixed with concurrent traces.
Removes option to run without durable.
Generates durable name base on subject.
Removes issue where the durable would not see every messages.
Motivation and Context
This change repurposes max_inflight. When > 1 messages will be processed concurrently.
This is a difference from before where all messages were processed sequentially.
Using max_inflight this way has been discussed.
How Has This Been Tested?
Existing unit tests passed. (output below)
I started openfaas in minikube on my mac.
faas-nets queueworker-dep.yml was modified as follows:
Once openfaas was up and running, I deployed cows function and executed the following command:
for x in {1..100}; do curl 'http://127.0.0.1:31112/async-function/cows' -d "{}" & ; doneAfter all curl commands finished I checked the queue workers log. It shows 10 functions starting/finishing concurrently: (This is only an excpert, full log of unit test/)
Types of changes
Checklist:
git commit -sSuggested documentation change
Parallelism
By default there is one queue-worker replica deployed which is set up to run a single task of up to 30 seconds in duration.
Full test log