temporalio / roadrunner-temporal

Temporal PHP-SDK Host Process plugin for Roadrunner
MIT License
22 stars 8 forks source link

[🐛 BUG?]: MaxConcurrentActivityExecutionSize ignored. #193

Closed Zylius closed 2 years ago

Zylius commented 2 years ago

No duplicates 🥲.

What happened?

MaxConcurrentActivityExecutionSize seems to be ignored, it's supposed to decrease the https://docs.temporal.io/docs/operation/how-to-tune-workers/#metrics task_slots_available metric, but it doesn't. On go SDK it's working.

In both go workers and PHP workers the default is 1000, but if I set it to 1 on PHP the metric stays 1000, when I set it to 1 on pure GO worker, it goes to 1 as it should.

I'm sure from the logs, RR is getting the right value.

Any ideas guys? Maybe I'm setting up something wrong? :thinking:

Version

2.8.4

Relevant log output

2022-03-22T19:12:33.968Z    DEBUG   temporal        received message    {"data": "\n\ufffd&*\ufffd&\n\ufffd\r\n\u0016\n\u0008encoding\u0012\njson/plain\u0012\ufffd\u000c{\"TaskQueue\":\"test_queue\",\"Options\":{\"MaxConcurrentActivityExecutionSize\":1,\
"WorkerActivitiesPerSecond\":0.0,\"MaxConcurrentLocalActivityExecutionSize\":0,\"WorkerLocalActivitiesPerSecond\":0.0,\"TaskQueueActivitiesPerSecond\":0.0,\"MaxConcurrentActivityTaskPollers\":0,\"MaxConcurrentWorkflowTaskExecutionSize\":0,\"MaxConcurrentWorkflowTaskPollers\":0,\"StickyScheduleToStartTimeout\":null,\"WorkerStopTimeout\":null,\"EnableSessionWorker\":false,\"SessionResourceID\":null,\"MaxConcurrentSessionExecutionSize\":1000},\"Workflows\":[....]
{\"TaskQueue\":\"test_conversion\",\"Options\":{\"MaxConcurrentActivityExecutionSize\":1,\
"WorkerActivitiesPerSecond\":0.0,\"MaxConcurrentLocalActivityExecutionSize\":0,\"WorkerLocalActivitiesPerSecond\":0.0,\"TaskQueueActivitiesPerSecond\":0.0,\"MaxConcurrentActivityTaskPollers\":0,\"MaxConcurrentWorkflowTaskExecutionSize\":0,\"MaxConcurrentWorkflowTaskPollers\":0,\"StickyScheduleToStartTimeout\":null,\"WorkerStopTimeout\":null,\"EnableSessionWorker\":false,\"SessionResourceID\":null,\"MaxConcurrentSessionExecutionSize\":1000},\"Workflows\":[{\"Name\":\"ConverterWorkflow\",\"Queries\":[],\"Signals\":[]}],\"Activities\":[....]
rustatian commented 2 years ago

Hey @Zylius 👋🏻 . I'll verify and fix it in the next release.

rustatian commented 2 years ago

Hey @Zylius. I tried to verify that, but as far as I see, I definitely can manipulate this argument: image PHP side: image

As you may see on the first screenshot, the ActivityExecutionSize is 10 as I set and WorkflowTaskExecutionSize is 9 because 1 WF was in progress to grab the metric.

rustatian commented 2 years ago

It would be best to wait for a little because this is not an immediate metric.

Zylius commented 2 years ago

Hmm I'll try to do it just like you did, maybe my code was wrong :thinking:

Zylius commented 2 years ago

I can confirm this was an issue on our end :( terribly sorry, closing the issue.

rustatian commented 2 years ago

Np at all :)