Skip to content

[Scheduler] Recuring messages not dispatched if cache TTL is less than the message frequency. #52911

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

Open
bendavies opened this issue Dec 6, 2023 · 11 comments

Comments

@bendavies
Copy link
Contributor

bendavies commented Dec 6, 2023

Symfony version(s) affected

6.4.1/7.0.1

Description

There is a bug in the Symfony Scheduler component where the cache TTL is erroneously set to a value less than a Scheduler's Message Frequency.

This causes the Scheduler to not dispatch any recurring messages at all.

Using a Cache with a TLL less than a Scheduler's Message Frequency is obviously wrong.
There should be no TTL at all, but it seems Scheduler should be able to handle this the same as a cache miss at continue producing messages as if there was no cache used.

How to reproduce

Follow reproduction steps at https://github.com/bendavies/symfony-bug-52911

Possible Solution

Not really sure?

  1. Fix the bug so a cache miss (from a ttl expiry) does not stop recuring messages being generated.
  2. Warn if the provided cache has a default ttl? (should be zero for scheduler?).
  3. manually set $item->expiresAfter(0) at
    [$this->time, $this->index, $this->from] = $this->cache->get($this->name, fn () => [$now, -1, $now]) + [2 => $now];
    ?

Additional Context

No response

@valtzu
Copy link
Contributor

valtzu commented Dec 6, 2023

You are right that Scheduler in stateful mode only works correctly with non-expiring cache.

Option 3 of your suggested solutions would sound the best to me, though I'm not super familiar with the cache component and the behavior of different implementations/providers.


Of course cache by definition is usually considered something you can clear any time without affecting business logic – which brings up the question: is using cache for the Scheduler state a good idea in the first place.

@kbond
Copy link
Member

kbond commented Dec 6, 2023

Of course cache by definition is usually considered something you can clear any time without affecting business logic

This is a good point. I guess the simplest solution would be to configure it to use a scheduler-specific cache.

@Jeroeny
Copy link
Contributor

Jeroeny commented Dec 9, 2023

Good find, I would agree that this could be improved. I am looking into the different proposed solutions. Some thoughts:

  1. The logic that causes the message to not be yield, is here https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/Scheduler/Generator/MessageGenerator.php#L72. It's not yield because the fact that the scheduled-time is before the checkpoint means that another scheduler process should have picked it up, hence the Checkpoint has passed it. The other part is the Checkpoint that sets its own $time from cache (also on miss). I guess this also cannot be removed because if the cache has been intentionally reset, you'd want it to start from $now. Though it could become a configurable option, like reset_on_cache_miss.
  2. This could be interesting. I don't think it has to check the defaultLifetime. Instead the Checkpoint could warn/throw when a cache was previously hit AND a cache miss occurs. Because this either means: a TTL caused the cache to reset (possibly undesired) or the cache has been reset manually on purpose. To still support the latter option, maybe a configuration value can be introduced: throw_on_cache_miss.
  3. This would remove the ability to still use a TTL. I think it still has a usecase: when you want a maximum time for the scheduler to be offline after which it should not resume from so long ago.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@bendavies
Copy link
Contributor Author

bendavies commented Jun 10, 2024

yes this bug still relevant.

@carsonbot carsonbot removed the Stalled label Jun 10, 2024
@mba242
Copy link

mba242 commented Oct 11, 2024

I am completely new to the subject of schedulers and may have a few problems understanding them, but according to Messenger: Deploying to Production it makes no sense to define long time intervals with the current behavior.

Don't Let Workers Run Forever
Some services (like Doctrine's EntityManager) will consume more memory over time. So, instead of allowing your worker to run forever, use a flag like messenger:consume --limit=10 to tell your worker to only handle 10 messages before exiting (then the process manager will create a new process). There are also other options like --memory-limit=128M and --time-limit=3600.

Maybe there is a solution using some cache, which fixes #58124 at the same time. Or alternatively a --date parameter like in debug:scheduler command for messenger:consume, which then processes outdated jobs first. So it is possible to implement a cache by yourself.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@bendavies
Copy link
Contributor Author

still a bug.

@carsonbot carsonbot removed the Stalled label Apr 12, 2025
@upgrader-dev
Copy link

Same here. Any update planned soon?

@fabpot
Copy link
Member

fabpot commented May 17, 2025

@upgrader-dev Someone would need to work on it and submit a pull request. Would you be interested in giving it a try?

@upgrader-dev
Copy link

Thank you but I'm just a beginner with Symfony. I'll give it a try anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants