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

Changes made in admin not visible after save (until the page is refreshed) if Redis caching is enabled #16146

Closed
mvarblow opened this issue May 23, 2024 · 7 comments · Fixed by #16147
Labels
Milestone

Comments

@mvarblow
Copy link
Contributor

Describe the bug

If I enable the Redis caching module and then make a change in the site admin, the change is saved but not reflected immediately. Specifically, this behavior seems to be consistent when changes are made to a role or to audit trail settings. In both cases, after POSTing the change, Orchard Core redirects to a GET endpoint. The "document" that's rendered by the GET endpoint does not reflect the update that was applied by the POST. However, if I refresh the page, then it does reflect the change.

I noticed this problem when trying to add a simple database cache (IDistributedCache) to avoid the Redis requirement when deploying multiple instances. The problem manifested but it didn't seem to be caused specifically by the IDistributedCache implementation I created (very simple). So I cloned the OrchardCore dev branch this morning and fired it up using the Agency theme with Redis configured in appsettings. The same problem occurs with the Redis distributed cache feature enabled. So it doesn't seem to be the IDistributedCache implementation that's the problem. It must be something about how OrchardCore is using the cache.
 

To Reproduce

  1. Clone the OrchardCore dev branch.
  2. In OrchardCore.Cms.Web\appsettings.json, uncomment the OrchardCore_Redis section and configure it to use a local (or cloud) redis installation. I used podman with the docker.io/redis container image.
  3. Start OrchardCore and complete setup. I used the Agency theme.
  4. Sign in to the OrchardCore admin and enable the Redis Cache feature.
  5. Go to the Roles page.
  6. Click edit on a role (e.g. Moderator). Change the role description (e.g. from Moderator to Moderators).
  7. Press Save. You're taken back to the Roles index. Notice that the role description has not changed. It still shows the original value (e.g. Moderator).
  8. Refresh the page. Notice that the role description now does reflect your change (e.g. Moderators).

If you enable the Audit Trail feature then the problem is even more pronounced on the audit trail settings page.

  1. Uncheck a few boxes and press save.
  2. The response in this case, after POSTing the changes, is a redirect to a GET of the audit trail settings. It should reflect your changes. It does not. This behavior is even more confusing for the user on this page.

Expected behavior

After making a change in the admin, the change should immediately be reflected on the page that's returned.

Screenshots

Roles list before making changes:
image

Edit the role description:
image

Saved the change; the change is not reflect in the response:
image

Refresh the page; the change is now reflected in the response:
image

Initial audit trail settings:
image

Make some changes. Before pressing save it looks like this (for example):
image

Press Save. Your changes appear to be lost:
image

You could easily save again and lose your changes. Or you could refresh the page and suddenly your changes appear:
image

@mvarblow
Copy link
Contributor Author

By the way, the same behavior exists in 1.8.3. That's where I initially saw the problem before I tried to reproduce it (see above) in the dev branch with the latest commits.

@mvarblow
Copy link
Contributor Author

Another note: this problem is difficult to debug. For example, just setting a breakpoint here is enough to make the problem not reproduceable:

image

Stepping through the code in general seems to make the problem go away. It seemed to me that the above line of code must be executing too late. I was surprised to find that if I only break at this one location then I still cannot reproduce the problem. In fact, even if I move my breakpoint down after that line, I still can't reproduce the problem with the breakpoint set.

image

However, disable the breakpoint (run with no breakpoints active) and it happens for me every time.

@mvarblow
Copy link
Contributor Author

mvarblow commented May 23, 2024

It turns out that you can't reproduce it if you hit a breakpoint because of the 1 second in-memory cache.

image

If you break for more than a second then that cache times out and you read from the distributed cache which has been invalidated. The problem is that when OrchardCore invalidates the cache, it was only invalidating the distributed cache. It needs to also invalidate the in-memory cache. See my pull request which updates the DocumentManager.InvalidateInternalAsync method to invalidate both caches if there is a distributed cache in use.

@mvarblow
Copy link
Contributor Author

@Piedone I'm surprised you haven't run into this problem on your DotNest platform? I assume you must be using the Redis cache?

I could reproduce this consistently on the roles page with just a single instance, though I expect that the problem would also manifest on and off (or consistently with sticky session ARR) when running with multiple instances. With a single instance, it only goes away if things are slowed down (e.g. the redirect to the GET endpoint takes long enough that the 1s in-memory cache expires). With multiple instances, you wouldn't see the problem if the GET request was sent to another instance which didn't have a fresh copy of document in the local (1 second) in-memory cache. So it might happen less consistently with multiple instances but would still happen at least sometimes.

Reducing the synchronization latency to a few milliseconds (from the default 1 second) might work around the problem, though I wonder what other problems that might introduce and would prefer not to use that workaround.

@MikeAlhayek MikeAlhayek added this to the 2.0 milestone May 23, 2024
@Piedone
Copy link
Member

Piedone commented May 23, 2024

We don't use Redis with DotNest. Thanks for the fix, we checked it during today's triage meeting.

@mvarblow
Copy link
Contributor Author

@Piedone Interesting. May I ask, if you don't use Redis, have you found that you don't need it to run on multiple instances? Or perhaps you don't currently run individual tenants concurrently on multiple instances?

There doesn't seem to be any comprehensive documentation today on setting up Orchard Core to run on multiple instances. And I'd actually prefer to be able to do so without having to add another infrastructure component (e.g. Redis). I was planning to use a SQL Distributed Cache implementation (along with a SQL-based distributed lock and distributed signals implementation). But I'm curious about the experience others have had with running Orchard Core on multiple instances.

@Piedone
Copy link
Member

Piedone commented May 23, 2024

Since we use idle shutdown of tenants, the number of tenants actually running at a given time is a fraction of the total tenants. So, no horizontal scaling was needed (and in fact no automatic scaling at all).

Good point about not having to use Redis. This is not the first time this came up, so I opened #16149, please share your experiences there.

Also see #16046.

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

Successfully merging a pull request may close this issue.

3 participants