Skip to content

PoC: BUGFIX: Close base workspace during command simulation#5828

Draft
mhsdesign wants to merge 4 commits into
neos:9.0from
mhsdesign:bugfix/close-base-workspace-during-command-simulation
Draft

PoC: BUGFIX: Close base workspace during command simulation#5828
mhsdesign wants to merge 4 commits into
neos:9.0from
mhsdesign:bugfix/close-base-workspace-during-command-simulation

Conversation

@mhsdesign
Copy link
Copy Markdown
Member

Fixes deadlocks as per - see test: #5513
Should (TM) also fix: #5713

This change is a safeguard to ensure simulation is only executed one at a time per base-workspace.
Leveraging content stream closing via the eventstore we prevent any writes or other publish/rebase operation on the base workspace.

This means that only one user at time can Discard individual or Rebase their own workspace. Publishing to one common base-workspace was always by concept restricted as only one changeset can come first (ConcurrencyException: Expected version: 2625, actual version: 2626).

Before multiple users could rebase / discard individual simultaneously and might just end up being outdated still. Those users will now face one of these exceptions:

  • ContentStreamIsClosed: Content stream "live-cs-id" is closed.
  • WorkspaceRebaseFailed: Publication failed: "Content stream "live-cs-id" is closed." and 46 further conflicts

As effect of closing and also reopening the base workspace we cannot calculate the workspace state anymore correctly and end up being more often outdated due to the two additional events on the base which where added after the fork.

Problem, we close and reopen now even more often ...

... closing and reopening the base workspace might turn out even more fatal as the operation is not atomic. A suddenly dead connection WILL corrupt the system and leave live closed until manually forced open - which leaves us to provide tooling. Content stream closing IS NOT a good idea and must be removed eventually. See #5827

TODO:

  • Migrate new changesVersion column - needs value / default value for existing projects
  • Find out if changesVersion makes sense^^
  • Remove now obsolete bool hasChanges column and replace with version = changesVersion
  • Evaluate to rethrow ContentStreamIsClosed instead of wrapping as WorkspaceRebaseFailed

Upgrade instructions

Review instructions

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the correct branch:
    • If it's a bugfix, use the lowest maintained branch which has the bug
    • If it's a non-breaking feature, use the branch of the next version (might be either minor or major)
    • If it's a breaking feature it should typically go into the next major version
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@github-actions github-actions Bot added the 9.0 label May 16, 2026
$baseWorkspaceContentStreamVersion
);
$baseWorkspaceContentStreamVersion = $baseWorkspaceContentStreamVersion->next();
} catch (ConcurrencyException $concurrencyException) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't we retry in this situation?

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 this pull request may close these issues.

2 participants