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

FEATURE: Speedup content cache flush by using cte in findAncestorNodeAggregateIds #5261

Merged
merged 17 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1ee4ed2
TASK: Add docs to `CacheFlushingStrategy`
mhsdesign Sep 24, 2024
4db7389
TASK: introduce `ContentGraphInterface::findAncestorNodeAggregateIds`
mhsdesign Sep 24, 2024
127afd0
WIP: remove hacky loop detection in `findAncestorNodeAggregateIds` by…
mhsdesign Sep 24, 2024
18ea4ba
TASK: Remove apply event hack and change hierarchy relation's parent …
mhsdesign Sep 25, 2024
c01a83f
TASK: Prefer commands instead publishing events directly
mhsdesign Sep 25, 2024
1a644ee
TASK: Add behat test for `findAncestorNodeAggregateIds`
mhsdesign Sep 25, 2024
9801101
TASK: Add behat test for multiple parent node aggregates
mhsdesign Sep 25, 2024
16c0b30
FEATURE: Optimise `findAncestorNodeAggregateIds` to use CTE and sort …
mhsdesign Sep 25, 2024
3cdf712
WIP position ordering for sibling in `findAncestorNodeAggregateIds`
mhsdesign Sep 25, 2024
965cc34
TASK: Simplify code in `GraphProjectorCatchUpHookForCacheFlushing`
mhsdesign Sep 26, 2024
947c398
TASK: Do not sort `findAncestorNodeAggregateIds`
mhsdesign Sep 27, 2024
5c1f157
BUGFIX: Cannot replay "The source workspace missing does not exist"
mhsdesign Sep 27, 2024
66c5513
TASK: Remove obsolete join
mhsdesign Sep 30, 2024
ed605bd
Merge branch '9.0' into task/contentCacheFlusher-followup
Oct 23, 2024
2dbd3a0
TASK: Task rename alias in ancestor queries
mhsdesign Oct 27, 2024
902e08d
Merge remote-tracking branch 'origin/9.0' into task/contentCacheFlush…
mhsdesign Nov 4, 2024
d75afc0
TASK: Optimise `findAncestorNodeAggregateIds`
mhsdesign Nov 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,31 @@ public function iAddTheFollowingHierarchyRelation(TableNode $payloadTable): void
);
}

/**
* @When /^I change the following hierarchy relation's parent:$/
* @throws DBALException
*/
public function iChangeTheFollowingHierarchyRelationsParent(TableNode $payloadTable): void
{
$dataset = $this->transformPayloadTableToDataset($payloadTable);
$record = $this->transformDatasetToHierarchyRelationRecord($dataset);
unset($record['position']);

$newParentHierarchyRelation = $this->findHierarchyRelationByIds(
ContentStreamId::fromString($dataset['contentStreamId']),
DimensionSpacePoint::fromArray($dataset['dimensionSpacePoint']),
NodeAggregateId::fromString($dataset['newParentNodeAggregateId'])
);

$this->dbal->update(
$this->tableNames()->hierarchyRelation(),
[
'parentnodeanchor' => $newParentHierarchyRelation['childnodeanchor']
],
$record
);
}

/**
* @When /^I change the following hierarchy relation's dimension space point hash:$/
* @param TableNode $payloadTable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ Feature: Run projection integrity violation detection regarding root connection
| language | de, gsw | gsw->de |
And using the following node types:
"""yaml
'Neos.ContentRepository.Testing:Root':
superTypes:
'Neos.ContentRepository:Root': true
'Neos.ContentRepository.Testing:Document': []
"""
And using identifier "default", I define a content repository
Expand All @@ -22,45 +25,54 @@ Feature: Run projection integrity violation detection regarding root connection
| newContentStreamId | "cs-identifier" |

Scenario: Create a cycle
When the event RootNodeAggregateWithNodeWasCreated was published with payload:
When the command CreateRootNodeAggregateWithNode is executed with payload:
| Key | Value |
| workspaceName | "live" |
| contentStreamId | "cs-identifier" |
| nodeAggregateId | "lady-eleonode-rootford" |
| nodeTypeName | "Neos.ContentRepository.Testing:Document" |
| nodeTypeName | "Neos.ContentRepository.Testing:Root" |
| coveredDimensionSpacePoints | [{"language":"de"},{"language":"gsw"}] |
| nodeAggregateClassification | "root" |
When the event NodeAggregateWithNodeWasCreated was published with payload:
When the command CreateNodeAggregateWithNode is executed with payload:
| Key | Value |
| workspaceName | "live" |
| contentStreamId | "cs-identifier" |
| nodeAggregateId | "sir-david-nodenborough" |
| nodeTypeName | "Neos.ContentRepository.Testing:Document" |
| originDimensionSpacePoint | {"language":"de"} |
| coveredDimensionSpacePoints | [{"language":"de"},{"language":"gsw"}] |
| parentNodeAggregateId | "lady-eleonode-rootford" |
| nodeName | "document" |
| nodeAggregateClassification | "regular" |
And the event NodeAggregateWithNodeWasCreated was published with payload:
When the command CreateNodeAggregateWithNode is executed with payload:
| Key | Value |
| workspaceName | "live" |
| contentStreamId | "cs-identifier" |
| nodeAggregateId | "nody-mc-nodeface" |
| nodeTypeName | "Neos.ContentRepository.Testing:Document" |
| originDimensionSpacePoint | {"language":"de"} |
| coveredDimensionSpacePoints | [{"language":"de"},{"language":"gsw"}] |
| parentNodeAggregateId | "sir-david-nodenborough" |
| nodeName | "child-document" |
| nodeAggregateClassification | "regular" |
And the event NodeAggregateWasMoved was published with payload:
| Key | Value |
| workspaceName | "live" |
| contentStreamId | "cs-identifier" |
| nodeAggregateId | "sir-david-nodenborough" |
| newParentNodeAggregateId | "nody-mc-nodeface" |
| succeedingSiblingsForCoverage | [{"dimensionSpacePoint":{"language":"de"},"nodeAggregateId": null},{"dimensionSpacePoint":{"language":"gsw"},"nodeAggregateId": null}] |

When I change the following hierarchy relation's parent:
| Key | Value |
| contentStreamId | "cs-identifier" |
| dimensionSpacePoint | {"language":"de"} |
| parentNodeAggregateId | "lady-eleonode-rootford" |
| childNodeAggregateId | "sir-david-nodenborough" |
| newParentNodeAggregateId | "nody-mc-nodeface" |
And I run integrity violation detection
Then I expect the integrity violation detection result to contain exactly 1 errors
And I expect integrity violation detection result error number 1 to have code 1597754245

# Another error. One error per subgraph
When I change the following hierarchy relation's parent:
| Key | Value |
| contentStreamId | "cs-identifier" |
| dimensionSpacePoint | {"language":"gsw"} |
| parentNodeAggregateId | "lady-eleonode-rootford" |
| childNodeAggregateId | "sir-david-nodenborough" |
| newParentNodeAggregateId | "nody-mc-nodeface" |
And I run integrity violation detection
# one error per subgraph
Then I expect the integrity violation detection result to contain exactly 2 errors
And I expect integrity violation detection result error number 1 to have code 1597754245
And I expect integrity violation detection result error number 2 to have code 1597754245
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateClassification;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateIds;
use Neos\ContentRepository\Core\SharedModel\Node\NodeName;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName;
Expand Down Expand Up @@ -188,6 +189,19 @@ public function findParentNodeAggregates(
return $this->mapQueryBuilderToNodeAggregates($queryBuilder);
}

public function findAncestorNodeAggregateIds(NodeAggregateId $entryNodeAggregateId): NodeAggregateIds
dlubitz marked this conversation as resolved.
Show resolved Hide resolved
{
$stack = iterator_to_array($this->findParentNodeAggregates($entryNodeAggregateId));

$ancestorNodeAggregateIds = [];
while ($stack !== []) {
$nodeAggregate = array_shift($stack);
$ancestorNodeAggregateIds[] = $nodeAggregate->nodeAggregateId;
array_push($stack, ...iterator_to_array($this->findParentNodeAggregates($nodeAggregate->nodeAggregateId)));
}
return NodeAggregateIds::fromArray($ancestorNodeAggregateIds);
}

public function findChildNodeAggregates(
NodeAggregateId $parentNodeAggregateId
): NodeAggregates {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use Neos\ContentRepository\Core\Projection\ContentGraph\VisibilityConstraints;
use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateIds;
use Neos\ContentRepository\Core\SharedModel\Node\NodeName;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName;
Expand Down Expand Up @@ -183,6 +184,19 @@ public function findParentNodeAggregates(
);
}

public function findAncestorNodeAggregateIds(NodeAggregateId $entryNodeAggregateId): NodeAggregateIds
{
$stack = iterator_to_array($this->findParentNodeAggregates($entryNodeAggregateId));

$ancestorNodeAggregateIds = [];
while ($stack !== []) {
$nodeAggregate = array_shift($stack);
$ancestorNodeAggregateIds[] = $nodeAggregate->nodeAggregateId;
array_push($stack, ...iterator_to_array($this->findParentNodeAggregates($nodeAggregate->nodeAggregateId)));
}
return NodeAggregateIds::fromArray($ancestorNodeAggregateIds);
}

public function findChildNodeAggregates(
NodeAggregateId $parentNodeAggregateId
): NodeAggregates {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
use Neos\ContentRepository\Core\SharedModel\ContentRepository\ContentRepositoryId;
use Neos\ContentRepository\Core\SharedModel\Exception\NodeAggregatesTypeIsAmbiguous;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateIds;
use Neos\ContentRepository\Core\SharedModel\Node\NodeName;
use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
use Neos\ContentRepository\Core\SharedModel\Workspace\WorkspaceName;
Expand Down Expand Up @@ -108,6 +109,13 @@ public function findParentNodeAggregates(
NodeAggregateId $childNodeAggregateId
): NodeAggregates;

/**
* @internal
*/
public function findAncestorNodeAggregateIds(
NodeAggregateId $entryNodeAggregateId
): NodeAggregateIds;

/**
* @internal only for consumption inside the Command Handler
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ protected function publishEvent(string $eventType, StreamName $streamName, array
/** @var EventPersister $eventPersister */
$eventPersister = (new \ReflectionClass($this->currentContentRepository))->getProperty('eventPersister')
->getValue($this->currentContentRepository);
/** @var EventNormalizer $eventPersister */
/** @var EventNormalizer $eventNormalizer */
$eventNormalizer = (new \ReflectionClass($eventPersister))->getProperty('eventNormalizer')
->getValue($eventPersister);
$event = $eventNormalizer->denormalize($artificiallyConstructedEvent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ public function registerAssetChange(AssetInterface $asset): void
}
//

$nodeAggregate = $contentRepository->getContentGraph($workspaceName)->findNodeAggregateById($usage->nodeAggregateId);
$contentGraph = $contentRepository->getContentGraph($workspaceName);
$nodeAggregate = $contentGraph->findNodeAggregateById($usage->nodeAggregateId);
if ($nodeAggregate === null) {
continue;
}
Expand All @@ -69,30 +70,11 @@ public function registerAssetChange(AssetInterface $asset): void
$workspaceName,
$nodeAggregate->nodeAggregateId,
$nodeAggregate->nodeTypeName,
$this->determineAncestorNodeAggregateIds($contentRepository, $workspaceName, $nodeAggregate->nodeAggregateId),
$contentGraph->findAncestorNodeAggregateIds($nodeAggregate->nodeAggregateId),
);

$this->contentCacheFlusher->flushNodeAggregate($flushNodeAggregateRequest, CacheFlushingStrategy::ON_SHUTDOWN);
}
}
}

private function determineAncestorNodeAggregateIds(ContentRepository $contentRepository, WorkspaceName $workspaceName, NodeAggregateId $childNodeAggregateId): NodeAggregateIds
{
$contentGraph = $contentRepository->getContentGraph($workspaceName);
$stack = iterator_to_array($contentGraph->findParentNodeAggregates($childNodeAggregateId));

$ancestorNodeAggregateIds = [];
while ($stack !== []) {
$nodeAggregate = array_shift($stack);

// Prevent infinite loops
if (!in_array($nodeAggregate->nodeAggregateId, $ancestorNodeAggregateIds, false)) {
$ancestorNodeAggregateIds[] = $nodeAggregate->nodeAggregateId;
array_push($stack, ...iterator_to_array($contentGraph->findParentNodeAggregates($nodeAggregate->nodeAggregateId)));
}
}

return NodeAggregateIds::fromArray($ancestorNodeAggregateIds);
}
}
6 changes: 6 additions & 0 deletions Neos.Neos/Classes/Fusion/Cache/CacheFlushingStrategy.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@

enum CacheFlushingStrategy
{
/**
* All content changes in the content repository are persisted immediately and thus an immediate flush is also required.
*/
case IMMEDIATE;
/**
* Changes like from assets (changing a title) will only be persisted when flow is shutting down. Thus we delay the flush as well.
*/
case ON_SHUTDOWN;
}
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ public function onBeforeEvent(EventInterface $eventInstance, EventEnvelope $even
$this->scheduleCacheFlushJobForNodeAggregate(
$this->contentRepository,
$eventInstance->workspaceName,
$nodeAggregate
$nodeAggregate,
$contentGraph->findAncestorNodeAggregateIds($eventInstance->getNodeAggregateId()),
);
}
}
Expand Down Expand Up @@ -197,15 +198,17 @@ public function onAfterEvent(EventInterface $eventInstance, EventEnvelope $event
&& $eventInstance instanceof EmbedsContentStreamId
&& $eventInstance instanceof EmbedsWorkspaceName
) {
$nodeAggregate = $this->contentRepository->getContentGraph($eventInstance->getWorkspaceName())->findNodeAggregateById(
$contentGraph = $this->contentRepository->getContentGraph($eventInstance->getWorkspaceName());
$nodeAggregate = $contentGraph->findNodeAggregateById(
$eventInstance->getNodeAggregateId()
);

if ($nodeAggregate) {
$this->scheduleCacheFlushJobForNodeAggregate(
$this->contentRepository,
$eventInstance->getWorkspaceName(),
$nodeAggregate
$nodeAggregate,
$contentGraph->findAncestorNodeAggregateIds($eventInstance->getNodeAggregateId())
);
}
}
Expand All @@ -214,15 +217,16 @@ public function onAfterEvent(EventInterface $eventInstance, EventEnvelope $event
private function scheduleCacheFlushJobForNodeAggregate(
ContentRepository $contentRepository,
WorkspaceName $workspaceName,
NodeAggregate $nodeAggregate
NodeAggregate $nodeAggregate,
NodeAggregateIds $ancestorNodeAggregateIds
): void {
// we store this in an associative array deduplicate.
$this->flushNodeAggregateRequestsOnAfterCatchUp[$workspaceName->value . '__' . $nodeAggregate->nodeAggregateId->value] = FlushNodeAggregateRequest::create(
$contentRepository->id,
$workspaceName,
$nodeAggregate->nodeAggregateId,
$nodeAggregate->nodeTypeName,
$this->determineAncestorNodeAggregateIds($workspaceName, $nodeAggregate->nodeAggregateId)
$ancestorNodeAggregateIds
);
}

Expand All @@ -237,25 +241,6 @@ private function scheduleCacheFlushJobForWorkspaceName(
);
}

private function determineAncestorNodeAggregateIds(WorkspaceName $workspaceName, NodeAggregateId $childNodeAggregateId): NodeAggregateIds
{
$contentGraph = $this->contentRepository->getContentGraph($workspaceName);
$stack = iterator_to_array($contentGraph->findParentNodeAggregates($childNodeAggregateId));

$ancestorNodeAggregateIds = [];
while ($stack !== []) {
$nodeAggregate = array_shift($stack);

// Prevent infinite loops
if (!in_array($nodeAggregate->nodeAggregateId, $ancestorNodeAggregateIds, false)) {
$ancestorNodeAggregateIds[] = $nodeAggregate->nodeAggregateId;
array_push($stack, ...iterator_to_array($contentGraph->findParentNodeAggregates($nodeAggregate->nodeAggregateId)));
}
}

return NodeAggregateIds::fromArray($ancestorNodeAggregateIds);
}

public function onBeforeBatchCompleted(): void
{
}
Expand Down
Loading