-
-
Notifications
You must be signed in to change notification settings - Fork 223
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
WIP: FEATURE: Rework CR CatchUp mechanism #4988
Conversation
1138434
to
3a2fc46
Compare
as discussed in todays weekly
and introduce `neos/contentrepository-dbaltools` package
and introduce `neos/contentrepository-dbaltools` package
…b.com/neos/neos-development-collection into feature/4746-rework-catchup-mechanism
...and use `Doctrine\DBAL\Connection` directly
@@ -44,7 +44,6 @@ public function __construct() | |||
self::bootstrapFlow(); | |||
$this->contentRepositoryRegistry = $this->getObject(ContentRepositoryRegistry::class); | |||
|
|||
$this->setupCRTestSuiteTrait(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no longer needed, see
Line 88 in 9ee2fc8
protected function setupCRTestSuiteTrait(): void |
..to avoid status from showing a diff everytime
192256e
to
fc6ec5c
Compare
{ | ||
$store = new SemaphoreStore(); | ||
$factory = new LockFactory($store); | ||
$lock = $factory->createLock('catchup'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: This lock needs to be CR specific (i.e. include $this->id
)
@@ -153,6 +159,67 @@ public function projectionState(string $projectionStateClassName): ProjectionSta | |||
throw new \InvalidArgumentException(sprintf('A projection state of type "%s" is not registered in this content repository instance.', $projectionStateClassName), 1662033650); | |||
} | |||
|
|||
public function catchUpProjections(): void | |||
{ | |||
$store = new SemaphoreStore(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has to become an external dependency and/or coordinated with #4751
This already looks very promising and I like the direction this is taking! |
Thanks for the feedback, appreciated!
I added some more lines to the task list above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats a lot of stuff but looks "good" so far (considering this is WIP) ... i went through all php classes by reading except the last 20 with only the +-1 changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this file now? ^^
#print_r(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS));exit; | ||
#\Neos\Flow\var_dump('HANDLE ' . $command::class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wip
return new class { | ||
/** | ||
* @deprecated backwards compatibility layer | ||
*/ | ||
public function block(): void | ||
{ | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we were to merge this like this - which i think is not necessary - we should use an actual class so its inspectable via phpstorm :) But i dont think we need b/c layer for new code
$catchUpHook?->onBeforeEvent($event, $eventEnvelope); | ||
$projection->apply($event, $eventEnvelope); | ||
// TODO this should happen in the inner transaction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
todo over here :)
]; | ||
$projectionsAndCatchUpHooks[$projectionClassName]['catchUpHook']?->onBeforeCatchUp(); | ||
} | ||
#\Neos\Flow\var_dump('CATCHUP from ' . $lowestAppliedSequenceNumber->value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. lots of debug code and i guess todos?
Neos.ContentRepository.Core/Classes/Infrastructure/DbalClientInterface.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess this is now part of CheckpointHelper
? Good thing we have it in the Core and now our lower level package ^^
i just merged 9.0 into your pr. A few notes from my side:
|
This change was partially ported from neos#4988 Following things have been adjusted: - Removal of the step: The graph projection is fully up to date - remove lastCommandOrEventResult - Removal of the step: The documenturipath projection is up to date - Use injected Connection in tests instead of DoctrineDbalClient (this will be fully refactored in neos#4988)
This change was partially ported from neos#4988 Following things have been adjusted: - Removal of the step: The graph projection is fully up to date - remove lastCommandOrEventResult - Removal of the step: The documenturipath projection is up to date - Use injected Connection in tests instead of DoctrineDbalClient (this will be fully refactored in neos#4988)
This change was partially ported from neos/neos-development-collection#4988 Following things have been adjusted: - Removal of the step: The graph projection is fully up to date - remove lastCommandOrEventResult - Removal of the step: The documenturipath projection is up to date - Use injected Connection in tests instead of DoctrineDbalClient (this will be fully refactored in #4988)
This change was partially ported from neos/neos-development-collection#4988 Following things have been adjusted: - Removal of the step: The graph projection is fully up to date - remove lastCommandOrEventResult - Removal of the step: The documenturipath projection is up to date - Use injected Connection in tests instead of DoctrineDbalClient (this will be fully refactored in #4988)
This change was partially ported from neos/neos-development-collection#4988 Following things have been adjusted: - Removal of the step: The graph projection is fully up to date - remove lastCommandOrEventResult - Removal of the step: The documenturipath projection is up to date - Use injected Connection in tests instead of DoctrineDbalClient (this will be fully refactored in #4988)
This change was partially ported from neos/neos-development-collection#4988 Following things have been adjusted: - Removal of the step: The graph projection is fully up to date - remove lastCommandOrEventResult - Removal of the step: The documenturipath projection is up to date - Use injected Connection in tests instead of DoctrineDbalClient (this will be fully refactored in #4988)
This change was partially ported from neos/neos-development-collection#4988 Following things have been adjusted: - Removal of the step: The graph projection is fully up to date - remove lastCommandOrEventResult - Removal of the step: The documenturipath projection is up to date - Use injected Connection in tests instead of DoctrineDbalClient (this will be fully refactored in #4988)
This change was partially ported from neos/neos-development-collection#4988 Following things have been adjusted: - Removal of the step: The graph projection is fully up to date - remove lastCommandOrEventResult - Removal of the step: The documenturipath projection is up to date - Use injected Connection in tests instead of DoctrineDbalClient (this will be fully refactored in #4988)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as discussed in #5061 we want to keep the result thing for now and with it the return values here.
git checkout 9.0 -- Neos.ContentRepository.Core/Classes/Service/ContentStreamPruner.php
git checkout 9.0 -- Neos.ContentRepository.NodeMigration
…up-mechanism # Conflicts: # Neos.ContentGraph.DoctrineDbalAdapter/src/DoctrineDbalContentGraphProjection.php # Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Projection/ProjectionIntegrityViolationDetector.php # Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ProjectionContentGraph.php # Neos.ContentGraph.PostgreSQLAdapter/src/Domain/Projection/HypergraphProjection.php # Neos.ContentGraph.PostgreSQLAdapter/src/Domain/Projection/ProjectionHypergraph.php # Neos.ContentRepository.Core/Classes/Projection/ContentStream/ContentStreamProjection.php # Neos.ContentRepository.Core/Classes/Projection/Workspace/WorkspaceProjection.php # Neos.ContentRepositoryRegistry.DoctrineDbalClient/Configuration/Objects.yaml # Neos.ContentRepositoryRegistry.DoctrineDbalClient/Configuration/Settings.yaml # Neos.ContentRepositoryRegistry.DoctrineDbalClient/composer.json # Neos.ContentRepositoryRegistry.PostgresDbalClient/Configuration/Objects.yaml # Neos.ContentRepositoryRegistry.PostgresDbalClient/composer.json # Neos.ContentRepositoryRegistry/Classes/Command/ContentGraphIntegrityCommandController.php # Neos.ContentRepositoryRegistry/Configuration/Objects.yaml # Neos.ContentRepositoryRegistry/Configuration/Settings.yaml # Neos.Neos/Classes/PendingChangesProjection/ChangeProjection.php
- Neos.ContentRepositoryRegistry.DoctrineDbalClient | ||
- Neos.ContentRepositoryRegistry.PostgresDbalClient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Neos.ContentRepositoryRegistry.DoctrineDbalClient | |
- Neos.ContentRepositoryRegistry.PostgresDbalClient |
they have been removed again, my change can thus be reverted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing because most of this is obsolete and will be fixed with #5321 |
related: #4746
WIP because:
CatchUpHooks
withContentRepositoryHooks
#4992)ContentStream
projection intoContentGraph
#4980, FEATURE: ContentGraphAdapter for write side access #4979block
see https://docs.neos.io/guide/contributing-to-neos/event-sourced-content-repository/blocking-until-a-command-is-executed and related