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

ConcurrencyException with Sitemaps when saving two content items at once #15743

Closed
Piedone opened this issue Apr 12, 2024 · 4 comments · Fixed by #15777
Closed

ConcurrencyException with Sitemaps when saving two content items at once #15743

Piedone opened this issue Apr 12, 2024 · 4 comments · Fixed by #15777
Assignees
Labels

Comments

@Piedone
Copy link
Member

Piedone commented Apr 12, 2024

Describe the bug

With the Sitemaps module enabled, you can get ConcurrencyException when saving two content items that are in the sitemaps simultaneously.

To Reproduce

  1. Enable and configure the Sitemaps module.
  2. Publish two content items at the same time. This needs a slower webserver/DB server, or you can simulate it by adding Thread.Sleep(5000); to SitemapManager.UpdateSitemapAsync().
  3. Experience "ConcurrencyException: The document could not be updated as it has been changed by another process"

Expected behavior

Both publishes just work.

This is because ContentTypesSitemapUpdateHandler runs for every publish and updates the same single document. I guess the same issue can happen with simultaneous content item removals due to the same reason.

This can be fixed by putting sitemap updates in SitemapManager within a distributed lock, and instead of starting updates from a content handler, just adding a deferred task and doing the update there. So, updates would be synchronized without an adverse effect on the UX.

Screenshots

image (1) (4)

@MikeAlhayek
Copy link
Member

@Piedone can you please submit a PR for this?

Too many 1.9 related issues that should be soon addressed.

@Piedone
Copy link
Member Author

Piedone commented Apr 15, 2024

Yeah, I intend to work on this soon. Is this also a post-1.8 regression though?

@MikeAlhayek
Copy link
Member

1.8 has been out for a while and I don't hear people screaming over it. We could back port it into 1.8 release branch and release 1.8.3 if needed.

I think once you submit a PR. We could release 1.8.3 if we end up not release 1.9 soonish

@Piedone
Copy link
Member Author

Piedone commented Apr 16, 2024

Ah OK, just because you were asking about a PR in context of 1.9 issues.

But now it comes to me that this is definitely not a post-1.8 regression, since I have this with a client project on 1.7 (but the affected code is the same in main too).

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