!!! FEATURE: Allow workspaces to be deactivated, leaving them unusable until being activated again

[sachera]

This is meant to address Neos performance issues when publishing changes.

This change modifies the database (workspace::currentContentStreamId becomes nullable) -> that's why it is marked as BREAKING

Basic Idea

Each Neos account has its own workspace and each workspace its own content stream. These content streams result in a potentially large number of entries in the content repositories hierarchyrelation table, which in turn, will make the fork operation slower.

Core Observation: In a lot of cases only a small amount of these accounts is actually actively used, i.e. have unpublished changes.

The basic idea of this PR is to remove the unnecessary lines from the hierarchyrelation table:

• Workspaces can be deactivated, which will remove the associated content stream, but leave the workspace itself in the database.
  • This can only be done if the workspace is both up to date and not used as a base workspace for another workspace.
• This will reduce the time a fork operation requires and therefore speed up the publish process.
• When activated again, a new content stream is forked from the base workspace and associated to the workspace. This can be done on login, opaque to the user logging in.

Implementation Details

The feature is exposed through commands in the WorkspaceCommandController:

• ./flow workspace:deactivate: deactivate a workspace (only if contains no changes + no other one depending on it)
• ./flow workspace:activate: re-activate a workspace
• ./flow workspace:deactivateStale: deactivate all stale user workspaces. A workspace is stale if:
  • it is a user workspace
  • contains no pending changes
  • has no other workspace depending on it
  • and the user the workspace belongs to did not log in for a given amount of time (by default 7 days).

The last command is meant to be used in a cron job or similar to minimize the amount of active workspaces without the need to manually deactivate workspaces.

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• 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

Related: #4388

[skurfuerst]

Hey everybody,

this change was done by my colleague @sachera in close coordination with me. I think it will help us a lot to improve perceived performance. Curious about your reviews @bwaidelich @nezaniel @kitsunet @Sebobo @dlubitz @mhsdesign :)

FYI @sachera is working on getting the linter green, just some final adjustments.

All the best,
Sebastian

[skurfuerst]

from @mhsdesign:

Thanks for that you two:) Great to see some new wind and new problems being solved!

Ill have to review this in depth but i read the description. There seems now explicit activation and deactivation involved. Do you remember the idea of forking a workspace during the first write operation on it instead? And respectively after a full publish a workspace would remove its contentstream again and all read operation happen on the base workspace? I was just wondering if this wouldnt make the workspace being used from the outside much simpler and we dont need the cronjob.

Because if now a stale user logs in id assume their workspace is reactivated which causes a new fork event and which slows down the login to Neos ui so much that its considered frozen. Also the user might not even want to edit content if its an administrator but might just want to manage users in the user module - so preparing the workspace is not always sensible. Creating the fork only at first time of write can be better visualised via ajax and a dialog with loading indicator.

[skurfuerst]

Hey @mhsdesign :)

Do you remember the idea of forking a workspace during the first write operation on it instead?

Yes, we also evaluated this, and the following reasons were in our opinion against this:

• currently, the login rebases anyways in case your user workspace is stale, so it is not worse than today in terms of login time.
• I find it a difficult pattern if a write is often fast (i.e. if the content stream already exists), but sometimes slow (if fork is needed) - hard to reason about, hard to debug etc :)
• Implementation complexity - the other solution would be much more difficult to implement (though we could do it later) IMHO

-> and we tried to find a good pragmatic, but still fitting to the existing architecture, solution :)

All the best, Sebastian

[Sebobo]

Hi thx for coming up with that!

Though it feels a bit more like a bandaid this way for a problematic situation. Most projects wouldn't activate the cron job probably, as it is "fast" in the beginning and only over time when they add more users or create new workspaces, it becomes slower and maybe nobody would think that this is the reason or the customer wouldn't call the agency, to say: hey turn of on the workspace disabling 😉. I hope you know what I mean. So this is like a very niche "pro"-feature IMO.

I already asked in previous discussions, why we need to update all workspaces all the time. This change goes into the same direction. So my question would be now: Why can't we detect "stale" workspaces on-the-fly?
I anyway wanted to mark stale workspaces as I have done with my Workspace enhancement in https://github.com/Sebobo/Shel.Neos.WorkspaceModule. This helped editors or admins to see in the workspaces list which workspaces are old and can be removed. Sadly we couldn't implement this feature for 9.0 as we ran out of time and some data was missing to define a "stale" workspace at the time.

Maybe we can use the "stale" mechanism for both of these things, but have a smarter way of when to set the "stale" flag or whatever we use for that.

And an important note: always make sure to handle duplicate events during login, as the login takes time, people reload or whatever and we've had some issues in the past with that and had to fix the event stream by hand on neos.io.

[mhsdesign]

@skurfuerst thanks for your response

currently, the login rebases anyways in case your user workspace is stale, so it is not worse than today in terms of login time.

id say we should not praise our current solution. A rebase, reactivation or any heavy interaction during the backend load is disruptive for our users. The solution works for a small demo but already with our Neos.io a login takes many seconds before succeeding. Id really like to find a good solution and it should be in line on what we work on here.

I find it a difficult pattern if a write is often fast [...] but sometimes slow

granted, that might be too much of magic for the low level content repository - primarily because due to the implementation time is a big factor. Otherwise id say from an API it would fit our workspace to content stream design.

---

Overall i agree that workspaces where there hasnt been any write activity on should not explicitly store all hierarchy relations in the database.

An process where the outer layers e.g. Neos.Ui heavily interact with the CR would be:

new user created

• user exist and can login without any slow sync operations
• opening the backend (always neos/content by default) shows the content of the "live" workspace, no workspace is created at this point
• navigation in the backend to other modules works smoothly
• attempting a write operation in the Neos Ui will prompt the user to create a workspace based on "live"
• editing continues in the users editing session
• after publish the workspace gets a new forked content stream
  • an optimisation could be to reduce work done within one server operation to make the forking after publish optional and another interaction for the user? If a user doesnt have additional work to do but desires to log out instead forking a content stream now to have to rebase it tomorrow would be unnecessary work?

user becomes stale

• if the user doesnt write to the workspace for some time we can remove the users workspace
• login even after being inactive does not cause disruptive freezes
  • and user is also not attempting to reload login page which had caused issues (see seb)
• the user can continue to view the backend in the base workspace "live" but before attempting any changes we re-request a new workspace as a manual interaction from the user (see above)

TLTR: In Neos 9 we removed the hard coupling from the workspace to user relation. So if we want to implement some workspaces that are not real workspaces we could just remove the workspace and in other parts of the system - e.g. NeosUi handle that by reading from the base. That way we wouldnt break anything in our Core layers, e.g. no new Events/Commands/... changed read models. It would be the total opposite of what i described earlier with the magic Workspace model which might or might not own a content stream.

Lets see where the discussion will take us ^^

[Sebobo]

IMO "Workspace becomes stale" > "User becomes stale". If we only check the personal workspace a project might still have 200 non-personal workspaces. Neos itself could then do some additional magic to make the personal workspaces more responsive if needed e.g. refresh them when it can anticipate the need.

[bwaidelich]

I'm a bit torn between a proper™ solution like suggested by @mhsdesign and @Sebobo and some intermediate solution that improves the situation earlier..
Personally I would love to see a more "sustainable" way – but only if it can (and is) done in a reasonable amount of time.
On that note this might be the wrong place to discuss alternatives. Nevertheless I want to drop one more idea that just came to my mind:
I remember that we once had the idea to keep "a stash of free content streams" in order to speed up performance of rebase operations. Maybe we could follow that approach here as well:

• A content stream for all non-personal workspaces
• A (configurable) amount of content streams that are kept up-to-date but are not assigned to a workspace
• When a fresh content stream is needed (i.e. rebase or user starts to edit) one of the stash is used
• Once the number of unused content streams fall underneath a (configurable) amount, new streams are created (potentially async and/or via cronjob)

[skurfuerst]

I agree with all your thoughts generally, though I very much suggest to improve the situation now/soon; and that is why I uphold the PR suggestion above as the imho best way to go forward with right now.

Imho we can combine this in the next steps with content streams prepared for later usage etc.

I feel we sometimes fall into the trap "it must be the perfect solution, otherwise we rather have no solution at all"... Personally I won't be able to work on totally different options outlined here for the foreseeable future, so does anybody of you want to pick up this topic? :-)

Does anybody see specific problems with the PR above? I did not see any yet from the discussion so far.

All the best,
Sebastian

[bwaidelich]

Does anybody see specific problems with the PR above?

The only potential issue I see (after skimming over it) is the new event types (because it's a breaking change strictly speaking and because it makes this design choice harder to change in the future). I wonder if this could be made an implementation detail of the dbal adapter instead.

But I totally agree to your suggestion to rather improve the situation than trying to come up with the perfect solution that will never be done :)

[skurfuerst]

Hm @bwaidelich ,

Good question about the new event types. I was thinking that we need to make this behavior visible at the outside - because otherwise we would need to "magically" materialize content streams even before changing things. otherwise our logic for validating constraints and eventual consistency with sequence numbers will fail - at least if I am not mistaken.

My first idea was to make this completely transparent, but I could not pull this off in my head 😬

Curious about your thoughts :)
Sebastian

[mhsdesign]

This PR breaking the API and introducing new events as well as the schema adjustments is not something i would expect in a patch level release. Technically the PR looks good though i see already in code the new deactivated state results in some odd phpstan hints and added complexity.

Instead as pointed out above Neos.Neos could just remove the user workspaces when they are considered stale which leads to the same state we already have.

That means WorkspaceService::getPersonalWorkspaceForUser() would trow (as it does for a new user) and WorkspaceService::createPersonalWorkspaceForUserIfMissing() would need to be run.

We already have that state in the life cycle of a user - so before adding a whole new concept like shown here i think a few lines to remove the user workspace if stale would have the same effect and is not breaking at all and trivial to review.

To also fix the freeze in the login we probably have to go further like outline here #5718 (comment) but this change did not account for this as well. So as an equivalent solution i would suggest:

new user created

• user exist and can login
• opening the backend (always neos/content by default) results in a workspace being created createPersonalWorkspaceForUserIfMissing()
  • this is a slow operation
• editing is done right in the users workspace we have
• after publish the workspace gets a new forked content stream

user becomes stale

• (NEW) if the user doesnt write to the workspace for some time we can remove the users workspace
• relogin is slow as createPersonalWorkspaceForUserIfMissing() is triggered to create a new workspace

one part in this process is new the other steps already exists, and all backend modules need to handle already that a user doesnt own a workspace yet as Neos 9.0 doesnt provide a guarantee.

later we extend the process as i outlined above with the Neos Ui only creating the workspace if write was requested.

in performance the suggested flow is not worse than 9.0 (which is already bad:D) but also not worse or better than this pr.

There is one slight difference to removing and recreating a workspace than to reactivate and deactivate it. The additional neos workspace metadata like title, description, and roles are lost. So in case we hold that information dear we can possibly implement an activation or deactivation on Neos side. But for now the removal should do well:

This can be implemented as easy as adjusting deactivateStaleCommand to use WorkspaceService::deleteWorkspace().

I see how many adjustments in the core are needed to implement deactivation there. We put a lot of effort into moving Neos metadata outside the core to the Neos WorkspaceService to simplify it. I think if we need a concept like this it should be part of the Neos WorkspaceService but at the core its simplest if a Workspace exists or doesnt - we also didnt end up introducing virtual workspaces or names because there is just too much complexity involved.

Thank you both for drafting out this big pr with the pretty tests and all the things:) And shame on me for tearing it apart? I hope you find the proposed compromise a good start?