increase worker to 4096, enhanced monitor, process, and add redis cluster

[haristku]

Description:

enhance:

• number of queue tracking worker
• monitoring tool, processor
• redis cluster option

[haristku]

@snake14 , @AltamashShaikh fyi, the PR require to enabled pcntl php-extention, because i don't know why the register_shutdown_function wasn't work, maybe because the signal is delayed by the sleep statement or $processor->process(), idk, what do you think?

[snake14]

@snake14 , @AltamashShaikh fyi, the PR require to enabled pcntl php-extention, because i don't know why the register_shutdown_function wasn't work, maybe because the signal is delayed by the sleep statement or $processor->process(), idk, what do you think?

That brings up a good point @haristku . Since Matomo has to support non-Unix operating systems, like Windows, I don't think we can use pcntl in the application. What do you think @AltamashShaikh ?

[haristku]

That brings up a good point @haristku . Since Matomo has to support non-Unix operating systems, like Windows, I don't think we can use pcntl in the application. What do you think @AltamashShaikh ?

i'll try to find the work around... be right back

[haristku]

That brings up a good point @haristku . Since Matomo has to support non-Unix operating systems, like Windows, I don't think we can use pcntl in the application. What do you think @AltamashShaikh ?

@snake14 pcntl is no longer mandatory..

[haristku]

@AltamashShaikh i'll make some more code spacing and alignment adaptations, brb.. thank you.. 👍

[snake14]

@haristku @AltamashShaikh I was able to do some functional testing today and things didn't seem to work quite as expected. No matter how many times I run console queuedtracking:process, it wouldn't process any requests. Before these changes, simply running console queuedtracking:process without any options would process any requests. I could get this PR to process requests if I used the -c option, but only if I provided a value of 3 or greater and it wouldn't process any requests until it displayed the TRYING TO WIPE OUT THE QUEUE message.

As a separate note, I couldn't get multi-threading to work. However, I switched back to the current release and saw the same. My local configuration only processes requests if I have 1 worker for some reason.

Final note, we should try to get the tests passing if possible. Maybe it would be easier to split this into multiple PRs, like have the cluster changes in their own PR?

[snake14]

@haristku @AltamashShaikh I was able to do some functional testing today and things didn't seem to work quite as expected. No matter how many times I run console queuedtracking:process, it wouldn't process any requests. Before these changes, simply running console queuedtracking:process without any options would process any requests. I could get this PR to process requests if I used the -c option, but only if I provided a value of 3 or greater and it wouldn't process any requests until it displayed the TRYING TO WIPE OUT THE QUEUE message.

As a separate note, I couldn't get multi-threading to work. However, I switched back to the current release and saw the same. My local configuration only processes requests if I have 1 worker for some reason.

Final note, we should try to get the tests passing if possible. Maybe it would be easier to split this into multiple PRs, like have the cluster changes in their own PR?

I figured out the issue. I wasn't creating enough requests per queue. Once I lowered the number of requests per batch, it appeared to be working correctly, even with over 16 workers. So, I think the main issue is getting the automated tests passing. @AltamashShaikh would it be easier to merge the changes and then fix the builds? Would you like to do some functional testing as well before merging?

[AltamashShaikh]

@haristku @AltamashShaikh I was able to do some functional testing today and things didn't seem to work quite as expected. No matter how many times I run console queuedtracking:process, it wouldn't process any requests. Before these changes, simply running console queuedtracking:process without any options would process any requests. I could get this PR to process requests if I used the -c option, but only if I provided a value of 3 or greater and it wouldn't process any requests until it displayed the TRYING TO WIPE OUT THE QUEUE message. As a separate note, I couldn't get multi-threading to work. However, I switched back to the current release and saw the same. My local configuration only processes requests if I have 1 worker for some reason. Final note, we should try to get the tests passing if possible. Maybe it would be easier to split this into multiple PRs, like have the cluster changes in their own PR?

I figured out the issue. I wasn't creating enough requests per queue. Once I lowered the number of requests per batch, it appeared to be working correctly, even with over 16 workers. So, I think the main issue is getting the automated tests passing. @AltamashShaikh would it be easier to merge the changes and then fix the builds? Would you like to do some functional testing as well before merging?

@snake14 We should get the tests passed in this PR only

[AltamashShaikh]

@haristku We just had a chat on this internally, will it be possible for you to break this PR into 2-3 parts ?

1. getQueueIdForVisitor change
2. Redis cluster option
3. Changes for commands

This will help us to integrate this changes faster and fix all failing tests easily.

[haristku]

@haristku We just had a chat on this internally, will it be possible for you to break this PR into 2-3 parts ?

1. getQueueIdForVisitor change
2. Redis cluster option
3. Changes for commands

This will help us to integrate this changes faster and fix all failing tests easily.

sure, i'll split this PR into 3 parts.. so this current PR should be canceled right?

thanks...

[haristku]

@AltamashShaikh @snake14

i have created two new PRs to separate the parts. PR for "redis cluster" part will follow later, because i have to fix the already checked "Enable Redis Sentinel" issue first..

ty.

[AltamashShaikh]

@AltamashShaikh @snake14

i have created two new PRs to separate the parts. PR for "redis cluster" part will follow later, because i have to fix the already checked "Enable Redis Sentinel" issue first..

ty.

Thanks @haristku :+1:

[AltamashShaikh]

@haristku Closing this one as we already splitted the PR into smaller PRs