Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New counting thing (great to have!) is not working quite right... #464

Closed
bossanova808 opened this issue Apr 9, 2024 · 13 comments
Closed
Labels
bug Something isn't working

Comments

@bossanova808
Copy link

Bug Report

At the least the percentage calculation is wrong:

image

I also had a failure during the sendout, which doesn't normally happen. I have restarted and it is progressing slowly now...

Plugin Version

2.14.0

Craft CMS Version

Craft Pro 4.8.7

PHP Version

8.2.17

@bossanova808 bossanova808 added the bug Something isn't working label Apr 9, 2024
@bossanova808
Copy link
Author

I presume it has started another batch now - but yeah that percentage makes no sense to me:

image

@bossanova808
Copy link
Author

Definitely something broken in sending with this latest release, am seeing repeated failures and having to restart the job manually.

@bencroker
Copy link
Collaborator

bencroker commented Apr 9, 2024

As of Campaign 2.13.0, sendout jobs use Craft’s native job batching functionality. You should be seeing a batch number next to the job title, are you not? The percentage is calculated based on the progress for the current batch, whereas the numbers (X of Y) are based on the progress of the entire job. How would you expect this to work?
Source: https://github.com/craftcms/cms/blob/5dd93e900aabe883fd8bb7ee572601243ec92f4c/src/queue/BaseBatchedJob.php#L132-L135

Regarding the failures, what are you seeing in the logs? And are you using a daemonised queue runner to run queue jobs? See https://putyourlightson.com/plugins/campaign#queue-running-process

@bossanova808
Copy link
Author

Hi Ben

Not sure if I missed some release notes on this update - they would have been helpful here. The basic issue was the batch number was too high (1000) - and it appears this was leading to time-out (signal 7s in the logs). With the previous limits, this batch number was basically irrelevant in practise, in our case. I'll reduce the batch number something a lot smaller in future, but didn't really have the chance to do so here as I didn't see these changes announced.

I think there as a batch number, showing, yes, but the whole thing was confusing in that you're showing the percentage of one thing, and a running total of another. Combined with the fact that the job needed to be restarted about 10 times during the (~9000 contacts) run, meant the X of Y didn't make much sense either - as the Y was the new lower number of only pending-to-send-to contacts.

In terms of what I'd want/ mathematically expect - the running total to always show the total total, so that X of Y makes sense the whole way through (even if the job does have to be manually restarted). And I think the percentage without any context makes no sense, I'd prefer to see something more like:

Batch # (A of B, C%) Total (X of Y, Z%)
(on the job itself in the the queue runner)

And the (X of Y, Z%) in the sidebar of the control panel all through the job.

...I think that would be a lot clearer.

Thanks for considering!

@bossanova808
Copy link
Author

(and yes, I am using a deamonised queue runner)

@bencroker
Copy link
Collaborator

bencroker commented Apr 9, 2024

Like I said, sendout jobs use Craft’s native job batching functionality, so none of the progress indicators displayed are controlled by the plugin. Changes were documented in the changelog, see https://github.com/putyourlightson/craft-campaign/blob/2.x/CHANGELOG.md#2130---2024-03-25

If you can explain your expectations in a clearer way, then I think it would be worth creating a feature request at https://github.com/craftcms/cms/discussions on how to improve this.

Is there anything I can help with here? You didn’t mention what you found in the logs.

@bossanova808
Copy link
Author

I did - Signal 7s, indicating timeouts.

My apologies, though, I see I jumped past the notes for 2.13.0 on my last Campaign update - I just saw or took note of the 2.14 notes and didn't realise there was a pretty fundamental change in how send-outs are done or would have done more initial checking. (Looking at the notes now, though, I do think something so fundamental - changes to the send-out process - should have perhaps been highlighted more obviously even in the 2.13.0 notes though).

Odd my batch size was set at 1000, though, as I am pretty sure I'd never changed that, and 100 would have meant no (functional) issues here. In any case I have set it back to 100 now.

I'm trying to be clear - the info displayed doesn't make basic sense to a reader - it shows nether the total size of the job at various points, and the percentage that is presented is without any context, right next to an X of Y figure to which it does not relate, so it reads to any person with even basic maths as incorrect.

...e.g the second screenshot above - 13% (128 of 7303) - it's just wrong all together, really. The 7303 is not the complete total of the job (only the changed job after failure and restart), and the 13% next to 128 of 7303 is just so out of context as to be confusing (to me, at least). So the results it one doesn't know really know the overall progress without doing some mental math ('original total was 9000, so there hmm, 1700 sent plus these newly sent 128, with a bit over 700 thousand to go...`).

It probably seems trivial to you but send-outs to many thousands of people are inherently important/anxiety inducing things, and good info is very helpful...but this is definitely not good info, it's confusing info.

It makes sense overall that you've now decided to use the Craft queue system, I am sure, but one of the results is that quite important info for the user is now presented in a very unclear manner.

@bencroker
Copy link
Collaborator

Ah yes, you did, my bad.

Perhaps the changes should have been better highlighted, but I’m not sure what I could have written besides that the sendout process now uses Craft’s native job batching functionality. The changes in progress reporting were known to me, the confusion they convey was not.

Thanks for explaining your perspective better. It looks like the 7303 is indeed confusing. It is the total number of recipients remaining to send to, rather than the total number of recipients for the sendout in total. The reason it displays this number is that some sentouts are repeatable, so the total number of recipients for the sendout in total does not represent the current send. I understand the general confusion, and how this can cause anxiety, especially when sending to large lists.

If the main point of confusion is the overall progress indicator, then would it make sense to change the percentage to a fraction of the total number of job items, rather than the current batch? So instead of displaying 13% (128 of 7303) it would display 1% (128 of 7303).

@bossanova808
Copy link
Author

Yep, the issue with the total being wrong will likely go away (for me) when there are no sendout errors (due to the now lower batch number). But definitely I think the percentage should be 1% (128 of 7303) for clarity. In the end, all I really want to know is the overall progress in an easy/clear to read manner.

RE: the change-log - maybe just a note in bold saying it would be good to check your batch size value, but it's probably not a big deal as for most it will be set to 100 by default anyway, I presume I must have mucked about with that value at some point long in the past.

Thanks Ben!

@bencroker
Copy link
Collaborator

bencroker commented Apr 10, 2024

Can you show me what appears in the logs?

The change in batch size and delay really only affected people who overwrote the defaults, in which case the values were left as is. And this was in the changelog.

@bossanova808
Copy link
Author

bossanova808 commented Apr 10, 2024

Bunch of these:

2024-04-09 10:45:49 [console.ERROR] [Symfony\Component\Process\Exception\ProcessSignaledException] The process has been signaled with signal "7". {"trace":["#0 /home/forge/imagescience.com.au/releases/dev/vendor/symfony/process/Process.php(252): Symfony\\Component\\Process\\Process->wait()","#1 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2-queue/src/cli/Command.php(187): Symfony\\Component\\Process\\Process->run(Object(Closure))","#2 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2-queue/src/cli/Command.php(126): yii\\queue\\cli\\Command->handleMessage(15064, '...', 3600, 1)","#3 [internal function]: yii\\queue\\cli\\Command->yii\\queue\\cli\\{closure}(15064, '...', 3600, 1)","#4 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2-queue/src/cli/Queue.php(144): call_user_func(Object(Closure), 15064, '...', 3600, 1)","#5 /home/forge/imagescience.com.au/releases/dev/vendor/craftcms/cms/src/queue/Queue.php(191): yii\\queue\\cli\\Queue->handleMessage(15064, '...', 3600, 1)","#6 /home/forge/imagescience.com.au/releases/dev/vendor/craftcms/cms/src/queue/Queue.php(166): craft\\queue\\Queue->executeJob()","#7 [internal function]: craft\\queue\\Queue->craft\\queue\\{closure}(Object(Closure))","#8 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2-queue/src/cli/Queue.php(114): call_user_func(Object(Closure), Object(Closure))","#9 /home/forge/imagescience.com.au/releases/dev/vendor/craftcms/cms/src/queue/Queue.php(164): yii\\queue\\cli\\Queue->runWorker(Object(Closure))","#10 /home/forge/imagescience.com.au/releases/dev/vendor/craftcms/cms/src/queue/Command.php(101): craft\\queue\\Queue->run(true, 3)","#11 [internal function]: craft\\queue\\Command->actionListen(3)","#12 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array)","#13 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2/base/Controller.php(178): yii\\base\\InlineAction->runWithParams(Array)","#14 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2/console/Controller.php(180): yii\\base\\Controller->runAction('...', Array)","#15 /home/forge/imagescience.com.au/releases/dev/vendor/craftcms/cms/src/console/ControllerTrait.php(90): yii\\console\\Controller->runAction('...', Array)","#16 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2/base/Module.php(552): craft\\queue\\Command->runAction('...', Array)","#17 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2/console/Application.php(180): yii\\base\\Module->runAction('...', Array)","#18 /home/forge/imagescience.com.au/releases/dev/vendor/craftcms/cms/src/console/Application.php(91): yii\\console\\Application->runAction('...', Array)","#19 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2/console/Application.php(147): craft\\console\\Application->runAction('...', Array)","#20 /home/forge/imagescience.com.au/releases/dev/vendor/craftcms/cms/src/console/Application.php(122): yii\\console\\Application->handleRequest(Object(craft\\console\\Request))","#21 /home/forge/imagescience.com.au/releases/dev/vendor/yiisoft/yii2/base/Application.php(384): craft\\console\\Application->handleRequest(Object(craft\\console\\Request))","#22 /home/forge/imagescience.com.au/releases/dev/craft(13): yii\\base\\Application->run()","#23 {main}"],"memory":39559144,"exception":"[object] (Symfony\\Component\\Process\\Exception\\ProcessSignaledException(code: 0): The process has been signaled with signal \"7\". at /home/forge/imagescience.com.au/releases/dev/vendor/symfony/process/Process.php:435)"} 

@bencroker
Copy link
Collaborator

Hmm, not sure what is causing that. You might want to double check that the number of processes are limited to 1 in the daemon.

@bencroker
Copy link
Collaborator

Submitted a PR to improve this in craftcms/cms#14817.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants