Skip to content

fix(cache): make clear() redis cluster compatible#61559

Merged
provokateurin merged 1 commit into
masterfrom
fix/redis-clear-cluster-compat
Jul 15, 2026
Merged

fix(cache): make clear() redis cluster compatible#61559
provokateurin merged 1 commit into
masterfrom
fix/redis-clear-cluster-compat

Conversation

@solracsf

@solracsf solracsf commented Jun 23, 2026

Copy link
Copy Markdown
Member

Summary

clear() now uses SCAN + UNLINK instead of KEYS + DEL. Added a cluster-aware branch: iterates each master via _masters() and unlinks keys one at a time so every command stays within a single hash slot, so clear() now actually works as expected on a Redis Cluster.

Aditionnally, LoggerWrapperCache::ncad() now correctly delegates to ncad().

Note: The @psalm-suppress NullArgument, PossiblyNullArgument on the SCAN calls is required because the phpredis stub types the cursor as int though it must start as null.

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
@solracsf
solracsf marked this pull request as ready for review June 23, 2026 21:44
@solracsf
solracsf requested a review from a team as a code owner June 23, 2026 21:44
@solracsf
solracsf requested review from Altahrim, ArtificialOwl, leftybournes and provokateurin and removed request for a team June 23, 2026 21:44
@solracsf solracsf added 3. to review Waiting for reviews feature: caching Related to our caching system: scssCacher, jsCombiner... labels Jun 23, 2026
@solracsf solracsf added this to the Nextcloud 35 milestone Jun 23, 2026
@solracsf solracsf self-assigned this Jun 23, 2026
@susnux susnux added the community pull requests from community label Jun 24, 2026
);

return $this->wrappedCache->cad($key, $old);
return $this->wrappedCache->ncad($key, $old);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks unrelated to redis, so should better be on its on PR for logical separation.
At least in its own commit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, will open a separate PR.

@susnux susnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redis part looks good

@susnux

susnux commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

/backport to stable34

@susnux

susnux commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

/backport to stable33

@provokateurin
provokateurin enabled auto-merge July 15, 2026 06:06
@provokateurin
provokateurin merged commit 868f9b1 into master Jul 15, 2026
325 of 352 checks passed
@provokateurin
provokateurin deleted the fix/redis-clear-cluster-compat branch July 15, 2026 12:41
@susnux

susnux commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

/backport to stable32

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

Labels

3. to review Waiting for reviews community pull requests from community feature: caching Related to our caching system: scssCacher, jsCombiner...

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants