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

[pull] develop from frappe:develop #527

Merged
merged 40 commits into from
Jan 7, 2025
Merged

[pull] develop from frappe:develop #527

merged 40 commits into from
Jan 7, 2025

Conversation

pull[bot]
Copy link

@pull pull bot commented Jan 7, 2025

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.1)

Can you help keep this open source service alive? 💖 Please sponsor : )

ankush added 30 commits January 6, 2025 18:57
Also no need to check channel names?
One can set it in conf and it won't work as expected because pubsub
format is different
Picking 1024 keys assuming 4kb avg size. In practice most things are
smaller so 1024 should be good enough!

This will likely only affect multitenant deploy with many sites.
We can be more aggressive later, this is a good starting parting.
Imagine:
| client 1 | client 2 |
| ---      | ---      |
| rd(x)    |          |
|          |  wr(x)   |
| inv(x)   |          |
| rd(x)    |          |  <- This will end up using request specific cache!
This is required if we do NOLOOP
Drops redis connection per worker from 3 to 2!

Nothing wrong with doing this.
Similar to recent _SITE_CACHE change, it's not required.

Properly cleared keys will get invalidated anyway.
This get+conditional set is also slightly prone to data races, but
doesn't seem to be harmful as of now.

Rationale:
- To enable recorder one must first send a request, so this should be
  set long before.
- While enabling we can accidentally clear cache for another worker by
  invalidating it, but that is kind of acceptable behaviour. We ONLY set
  it to False when `None` is received from Redis. Local invalidations
  remove it completely.
ankush and others added 10 commits January 6, 2025 20:24
- This should not be in init.py
- set_single_value is not a good alternate for properly cleaning up cache.
These can be used for logging or debugging the behaviour of the cache.
These are just simple strings, and default of 128 isn't enough. Just 4-5
list views will fill that up.
feat(cache): server-assisted client-side caching
@pull pull bot added the ⤵️ pull label Jan 7, 2025
@pull pull bot merged commit 939f832 into fproldan:develop Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants