Closed matthiashanel closed 4 years ago
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.
Tip: if you only have one commit so far then run: git commit --amend --signoff
and then git push --force
.
@LucasRoesler PTAL
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 -> https://github.com/openfaas/nats-queue-worker/pull/95#discussion_r418503395
@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
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 "{}" & ; done
After 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 -s
Suggested 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