From cd45c58799bef51e62635c64eaecb2551f35bb8d Mon Sep 17 00:00:00 2001 From: Denny Lubitz Date: Sun, 1 Sep 2024 15:22:58 +0200 Subject: [PATCH 1/2] TASK: Improve performance on content cache flush --- ...phProjectorCatchUpHookForCacheFlushing.php | 29 ++++--------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php b/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php index 0cc027bfaef..008f0d24065 100644 --- a/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php +++ b/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.php @@ -110,7 +110,10 @@ * => INVARIANT_2 violated. * => this case needs a cache flush at onBeforeBatchCompleted. * - * SUMMARY: we need to flush the cache at BOTH places. + * SUMMARY: We need to flush the cache at BOTH places to ensure non of the cases above happens. BUT (!) + * Flushing the cache is a very expensive operation, so we need to reduce them as much as we can. + * So we decided to take the risk of CASE B, which might show an outdated content a bit longer than + * it might be expected. * * @internal */ @@ -118,20 +121,11 @@ class GraphProjectorCatchUpHookForCacheFlushing implements CatchUpHookInterface { private static bool $enabled = true; - /** - * @var array - */ - private array $flushNodeAggregateRequestsOnBeforeBatchCompleted = []; /** * @var array */ private array $flushNodeAggregateRequestsOnAfterCatchUp = []; - /** - * @var array - */ - private array $flushWorkspaceRequestsOnBeforeBatchCompleted = []; - /** * @var array */ @@ -257,7 +251,7 @@ private function scheduleCacheFlushJobForNodeAggregate( NodeAggregate $nodeAggregate ): void { // we store this in an associative array deduplicate. - $this->flushNodeAggregateRequestsOnBeforeBatchCompleted[$workspaceName->value . '__' . $nodeAggregate->nodeAggregateId->value] = FlushNodeAggregateRequest::create( + $this->flushNodeAggregateRequestsOnAfterCatchUp[$workspaceName->value . '__' . $nodeAggregate->nodeAggregateId->value] = FlushNodeAggregateRequest::create( $contentRepository->id, $workspaceName, $nodeAggregate->nodeAggregateId, @@ -271,7 +265,7 @@ private function scheduleCacheFlushJobForWorkspaceName( WorkspaceName $workspaceName ): void { // we store this in an associative array deduplicate. - $this->flushWorkspaceRequestsOnBeforeBatchCompleted[$workspaceName->value] = FlushWorkspaceRequest::create( + $this->flushWorkspaceRequestsOnAfterCatchUp[$workspaceName->value] = FlushWorkspaceRequest::create( $contentRepository->id, $workspaceName, ); @@ -297,17 +291,6 @@ private function determineParentNodeAggregateIds(WorkspaceName $workspaceName, N public function onBeforeBatchCompleted(): void { - foreach ($this->flushNodeAggregateRequestsOnBeforeBatchCompleted as $index => $request) { - $this->contentCacheFlusher->flushNodeAggregate($request, CacheFlushingStrategy::IMMEDIATE); - $this->flushNodeAggregateRequestsOnAfterCatchUp[$index] = $request; - } - $this->flushNodeAggregateRequestsOnBeforeBatchCompleted = []; - - foreach ($this->flushWorkspaceRequestsOnBeforeBatchCompleted as $index => $request) { - $this->contentCacheFlusher->flushWorkspace($request, CacheFlushingStrategy::IMMEDIATE); - $this->flushWorkspaceRequestsOnAfterCatchUp[$index] = $request; - } - $this->flushWorkspaceRequestsOnBeforeBatchCompleted = []; } public function onAfterCatchUp(): void From 02fb77a611b493a94b0de1806b0d9828715ad6df Mon Sep 17 00:00:00 2001 From: Denny Lubitz Date: Tue, 17 Sep 2024 17:35:24 +0200 Subject: [PATCH 2/2] TASK: Improve performance on content cache flush --- ...ojectorCatchUpHookForCacheFlushing.monopic | Bin 5629 -> 2798 bytes ...phProjectorCatchUpHookForCacheFlushing.php | 89 +++++------------- 2 files changed, 25 insertions(+), 64 deletions(-) diff --git a/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.monopic b/Neos.Neos/Classes/Fusion/Cache/GraphProjectorCatchUpHookForCacheFlushing.monopic index c7fc9e98bb532eaa918a3ec2eaa7e589e8f4275b..cfec39ec13c7097829fcf65ae39dd3bf7aa01b65 100644 GIT binary patch literal 2798 zcmVyF#F6@C>(zjgslz`Mi&x!@Y2C zq35cDS+tbL;L$67PN5S0RS~Id%1qx}UP|k7kXN;9wF&QJo1qr%e^YcN2P!#HEKj?T zdx5)`5g;X_`dM)7ar5X8u_=_5i$C(&B8hXMB>nS)SDq;(XX?bGmK(HaH1IoU6dcni zM4&}BgnY1YqGa!%a5mqFcq!!fdAbvxyG@p2GgD8jABITj+xu)bPtv7GrDirs>fRFM z2ap30$XjIVaG89X6)Kch$=`>09xm3Qlx^fH35(zEj$N;eBaH8Mn=swqMSkzOt2Eq- zSr{+YNm_eaUuk#WnBtbRd0O7JaZ+`rI=M*}zp2Mtg_}$q)7&~*E=6p9u*lbGvb$TG z6Z23uTb_!wQc>fah}C0OH^{@grruGdp=7lxI>`kdaP#14m=RrP(qp=(M-NS8OyuNquu>7%!Tt)nkc-G*5+inI`LhySvogAII;Nj zy=m+sNn_PLu)4nyH>jvv>E+-K!x|SIJ@VP}oR) zgeY}Y7E&Ap`hf2b0-z6|`@!638D{HBzjE9aJ-93WCi3s&p$0b%wW`Zax0JnCS_)D9 znXbZxP}o=9p&Sy;r_>^A)>e%t)m1aGjPhFl@AdHw^_h;rUzovu)O6s|7a-;>XwFiV5b$5`C7m{$@64e;#@gZRPb?o zy9o2e`qz8cXp-COB}y(%mB0Ms?d$jPPe1@C1IQeM6o!SgZ3DIKwiubTPCRkD$_z7x=ki6JkdNJZBBV;-^wh1P~;ES*Z}aL znte4v+S8A}$zMB4okyo~q$h5yCvFV&0a7z~L3ZOrGL}N~=vbeAWe)xd3CYBr8td|k zWur+|ONs^@A#62fv<>fTjlsliBERPa{wSE?rDtKf0F$t-G4Ie*l2fV|Eb{;YLJ2&E}<3) zwM?jmLM;_)u~5r}S}@d-k(P|KWK`nd>9F=CkbM=d8cqld>@XqZ_j7ItyEyzz;#aF& zq_0aB|33cH4~}?w_tIHycG=qb^Da{{f`#lJVIpkUPN&ee2pb(9VI$lc^9}m(%yL$; zLGrJNWJ1`^GEp1L^fi?E8YF!Ul0KP+WmYXAU0J5j?6XXtc43*slKbkzp4oxGqsF!F zn_-|HF@zHIZ-}&3Ek(%)H}_J1BCoNPO#D8JciXv03+=GtD^hfplivC903ED(Kcd=BBw1(hefh4kBXHu%W6YtScKrfqgb4 zoyQJrh}w&^AY0yXmx=-CgEK1`P#AiVH3Pv!H3Kiui<3(NAPed>0fc$*=}Q7i3n-6F z*o#$hFBAA;{&m#_s9APkVL)(sDcF;0Vl$6vC^|s;jqa~Z+MwJcw#g(+|*E0!%Ph^y*6GtoR&glF@8!dtts$P z3z;lZRD&{}#J81Rq$A7~WvbUofYJ}_RTOsO=j`nGIi=5Wkx}C!uuEK|$;9y_aS>{W zI5k9^8X`^&5vSKi#MxjX&Oqp2PEo@3_b#xx0pbJ&{FD*+nVtpZmscTL#Juf zH{FLlRu#C~TagJN>(@Yu-KrYVh~dKx8}Z?~qPCn=VomS}6)T;VfT~3{XGUCz|4dxQ zBrf$eM<){)+F0yKfEP?aV2B|r#E=zY%Al#KgQi!u;1klpD?Wn}xCgxS6;8A^1Tge? z*VBmfB zgY}klw7??L($4_lsRqbkrxIvnEeJ(|i_5>i`6zGkOP-|S_O?uTnsRMCJh|oDTl%5U zvc)w7gCn@OZBrODcuyGG$sZu>-24H|pL1$s`U9t|0sG&7{(HKwKuFzJz{EonB-%lA3Q-8{I|`q51Mz7z36IOXy^6SgL8JPr>v{{UD+ zs4~rOVX@a0Ip&!6Se>lqE)8y`b{(6%EuLsiPQ5XmSLLW>c=kg?xYN3vplw#!TW%gs52rd)oYj3P2`T>5yHZ?xO@n47M>X2fD~k}(;Q=CQ&o}9@dEM!y-j+7Eg2-MpiHr~g z@!Pyqf3TOTYE7+0DyY5PhH=kAN*`7AW`3tk3qHG&ar9g=KI#nmtzCvr&amJ{IXUKD zv5DkUXIRWuA!`5hY9USqdDWia*Og(xgX_t$KRor#EUjB&wsuvrII%-Z`3=s;D4Qb# zs;s$*G}GY|7PYupGO@U!G3>jetED(My}X@bPxAhpzx?{@=T~pvy?Nv0>o9kkr3KD9 z%$#%=J9Dv;7dlFlX|i!5c~;~*Ip-~%JdszH^P)&!noVczy;cNX;0rp`mOYPErL?Uf z9b8d1hd{4;GRJJB4b4v45LKIWPgU<*YfSfgU-T69zNk~ZZ{UxyBP+h9os(IQ++?8X zexTWYV78G2;+NO0G{0J&;B| z@`idh6Rc!6Wt^splbDxh=!7J>vVaGH>2uF5W5(TR_eWEHH&tp>M*dsGD?MBx-8AZq_rM+ zc&Z6lOb4~So(2zpJq_*|y<(U59tO3kjt(=AvKPL+w+q>oNpXfS2mbz1@^V$6n4x z$RwV`!Omb4G}JA2w`SB-uf&e0Cor&2vk$gUvZN?pQ614%k-VZ-)E}FcrBaEkx^+L! zJ?Hn){Oa%Vb-El~{GMOFaw5N)%&#vO@y+OB^%{)A(PX+<#_8xngri0F{>z8?a+*%( z$>;)HI7*)$;>+nI`v-=j^1m)`X5-szVE7oWhS4@3n%#l2Jr~^EqWR6u>YUxE{`~m- zX3e(lVE%D>0>y(W7KO?m-Qs3^9p_tl^9{xF-~OrPkd=kC**p1Dxr`^%wDPFeH{~<; zke-hH!#YUUJLHdB?b)(g+X#c7wOSh!1^ENY9$5Ci2tzYkk0Dh)#%!9z&u;qqM?Kq^ z^v0T}@@$*dYzn`-S7Qa~r|IoyjO$r0)2H0B$;uW`Q3&~6FL(cXZ+(}&J-|`T3IoivNjb~y8LqbUA58W0b7#F0!L81`<%c*f7S=S$rgb@sZ;GW{ zIeT~A-QwNpy!h;n8#$g$Z?>m^aFkJT$HtFT%yAIpK2ZDv?Nhl+j`a?MUyZ)Z*K>8t@n-ea46$Cj(Ht2U*kAcaD9rK=j-@pzKDNGzRvg0oB932EY6OMi(isolAGD%@-Dc} z3`tB!8948Uhx;-ycYy!(exPz|9X5gP0vhB6c{8H$&(!@s<86hn!D#ZVEpjiK@&naB7#$L2~{ zzQYKFmm$^e*bJ%0bKL;8mUcW>LNqpI!H7%I_&?Od&g;!4sL<)k#_q_i2B?bNod z?L?>8(&iYcMnN$s$!8l;)L5udn^}1`prS>D;mU{c zDq2V*C_DtZB3CH0*}W7ks*{nvMT>l!uoo?)Q9eZr)QV^8Dllxmm8o-MjE@%sySx~* zd`OBs8N-w(i`ZU&5&@05c=PV zI=IUHlkwt-LMFwBheABPyO_*BguyIMzgPz2$s`yjLH2sQNXD~Zkq4#AG)U(`dY8qh zPs=pE4}%Yjd2%~@3exF)Jh|9}s(h7yv|NdNuxyDNkWCK|o8z?!;@Er?JtD`{E3#I{ z25$g`XPj7Z?jtnHG8nw5qw!4SNT)%FpL^Oa*R?xCYjz z2NHSvTlN({rSnC6aZ#Lkn=(AtZ3IT(k>Eq00je%MR;Ad^4Jiy?CbUQt?ZUaO@aL^k z!%hw){FZ3ir_J)_)7C*xp0{m3i0Rc7 z3Ffm&Fd3)g;NQLquE)t2X^<_Ahd2rDvbP2=*KdDV%&+6+GPo*s`G)>LF@faJa1Csk z23F!=&apV~%6a|%+c&|FZ~yZ1+n?SCFRz}0d77<`;N3Ux%7yXeFG&`a<~_R8BpBz- zyG46{uxj-WgR93hxLOec1nFX&EXS)dHDzyKeHDC|&hk>tc(PfMFW>()$Sh^>6sPNV zX2-yn)px(T$^Xu)I{*H-Tn)JT(*OAO{Wrl6-~B20)9d%IU%Yrh1%y*aiRkJP2BGt= zV@o=%Bw@WIVZCHeAd%{1yg?$M4geAXiy+z%V9Kwdc|>Ts`0sSJyfp3lYkO z{$R9WwngBpkfLrlKNSMd1fwnrgo3L&RnTPYBs)>9-Aknz$xawKJdJKx1`%NvXp|4l z)!kax`p!yZQ^E!vJ+t~W1MGkJ9Mka)86kg6wo$UV7b_hvhj+l{u_ zK_q)%J3~ruAvML3*CxqplX7TGk+OB8tpX-OYg1?VOnbZMfX_#>$W^!N011Jr1F#1` zU5_+?XrE#O#@ry^b7x~=fjU~69Jtf=W=FM8GHU%KbAMs*g8wf*A2YL*gP*DRgs zpeMxG_MJUZ7oLJwi2(4}6n6pC*#a^kHr_4fe=i_|!7|NXc$tICUwsuM@dpo*5ZKb< zeFY)(AjVs)k>ZNFi-7RjKoDsdm>4f$YcXE1%^|EHDdSuKVRyD(>$cGvm;v=JC>O30 zG=7kAAo33&wIJFw5_*bbs*~}KVAE!O`$l z>rmAdtx&bbf*>+ZTu3g?fzzvBZmjRc#dT4{+#s*8nQ4@K1xe6+n=oRmd_yN(b`f?E zvB@sj%rqmbUeraom{D!D=$jZyDH@uTq6Ian32IUU0_{Lwt+SQM__xl&AzEj_t94dd z!PQqLx}ZE1wcRALeUFV`bLMyKsC7 zL=R8}L4^iE6fuGtdIUA}h=En-B($#1IomA81s_9$M<>LIkvSWhE)C7Dkab@xgWFo1 znZs2i)DLWCki-8 zuUrvm9~zD%C@dG$OCtzLE(p9WhSp0XTKCduFQK#O*mTY!JPU|-X)JbVzwB!(qAhu| zS2}#vL)#q*p+==qh{|9oDr2Q+V9I7udtoyh)e^e2=N{ODA8L0d3$i6v5z&dbs_$IH zSm+qZjhF}v9n-i{x!Oc*`?C0;h(v7@{FS4?&Qbddxm{B9R1K+4#=9CK#Q~}zQutLv z4C9`+rw5z)>2Ke^3Euti?f36DbbXewm3T79c_h7eB%cfW5}&V5##?+I zqXP(GqNn(LVeat9DTbbft8U1-i?W6jx$;ty&* z5FJXIH*0Scx}W0{`x<2Ez#R*#DgARZTaX~#)X$A((u z23r!jLl+$R($vn;w4bAeL8R0KV=rF^oIBt}8k_f_9vQsOoiQ!P%ejMY3*?eTn5GA` zoukaEqhzL|%&L=gH86||y4Dz%qK|WL^I?t+wCFa8otg*ng?kmtZsI+ly*EiH=j7l%tlukAs zrlu$G!e%Dk&;_IUGe*V_xZvtwdGO{^;!dF&@fKCR&yHLYfb}nK=&nGa~byyVGpBdxveXt+SYZ{qPb@neoq#b z`no-~j7n`83)(U&wLL|p)tPuYJoW%lsr8IX`DAXT?LviE10!uZ)J99KH#P#3$}iRm z@nU-+UJxCi+W@&V)|6=E4QhGDjD9Njr^^zVE-PddZR(ydqnnv{V@CA>$mO;5LyI-! z;%!0XSc##Kk|G8*E#X0PCiy1G(uj0LxTK0mC)0 zIz29edZ}BA2prov38JnnRS=oCOw0|a&*T$zsd{=8MUXp1kUK??JJplkUv)CxdVi=6 zp!bK$Pw&r*jSi1aTo+GNTvv%u^+R6y;}2hd_x8^LTm*}FkuQfuW^|WvW_^<&2m4RP z>G*&D`+xTL5uXd}iV$rNP9lkgA}Q=5DeNLC?BdWAcCiD6T`mp_Y$4rCvjcwQyKB+Q)?e_9g>Q7c|o^<1incFgd{*n z0fYoV%8_L`vMNVb<;WD+P$)rRjVec0<;bcWnNnPo;i3c=<+pS>vMzAV`v)3-^w~cd zKUB|t88PZl3)4ZR7x^Gts7ynJPDY(r$0sCQJNG?SzQWcSE=Iq8+|)eStY1^*Q8Xb?lLRsD!D|U6);jaMuXx=uBB41g&83^;z*u2k}IC~F7;C@s7%JcheaKthedhyuqeR@ z%JI|lgQL^)6X#C*PU{erE{y#v4O?yY62d;}45SN+qzj6qe2pYFol}4ftIhzeo9)rZ zMkqKoRv`svR-sO9#0z|}w{cLgMJ1$6*aK2cQ7TkXE>ux6R8clm!N7=w6tpH1vdx-c zwP~Do+Az}k*`;H{P^eQCJ@3sffrnjsrf@EMu}dUyu1Mfqkt3tXkx@evI9Kfj&Kvhy zRN$y+&oW7$NdZu8kV<$cH`T$uWRfbA@s~-G2Waa=>DAXMyP(-)E zB5uHl1FJbG(Yod!9iJAf;?j(_TpbSD=Mb2~{DU+uys) zXD9OyVK9r+FP6c0G6}{>kUjo*k&I`-BJYV^ra?OQYJbP*dHegSlkCv2v`wqrrdMv$ zEVt>F!=Xu7p=}=`cA&Z}BSvtoZpLpZB`4sQiOf?8;A;2wIV<4kG*RKKiQ4U1F;8CK zWM{*hoRaG2hu~%Auis6R+h8(J;{C-?VOtDU=@3O9^-yZtra%BFDgjbMg`ugTLVGn- z<6}3ZJvJXx+B0mqZ_I?W+vsH%>K new) │ │ TX commit │ - * │ │ (end of batch) │ - * update sequence │ │ - * number │ │ - * │ │ - * onAfterCatchUp - * onBefore (B) - * BatchCompleted - * (A) - * - * There are two natural places where the Fusion cache can be flushed: - * - A: onBeforeBatchCompleted (before ending the transaction in the projection) - * - B: onAfterCatchUp (after ending the transaction). - * - * We need to ensure that the system is eventually consistent, so the following invariants must hold: - * - After a change in the projection, some time later, the cache must have been flushed - * - at a re-rendering after the cache flush, the new content must be shown - * - when block() returns, ANY re-render (even if happening immediately) must return the new content. - * - (Eventual Consistency): Processes can be blocked arbitrarily long at any point in time indefinitely. - * - * The scenarios which are NOT allowed to happen are: - * - INVARIANT_1: after a change, the old content is still visible when all processes have ended. - * - INVARIANT_2: after a change, when rendering happens directly after block(), the old content - * is shown (e.g. because cache is not yet flushed). - * - * CASE A (cache flushed at onBeforeBatchCompleted only): - * - Let's assume the cache is flushed really quickly. - * - and AFTER the cache is flushed but BEFORE the transaction is committed, - * - another request hits the system - marked above with !1! - * - * THEN: the request will still load the old data, render the page based on the old data, and add - * the old data to the cache. The cache will not be flushed again because it has already been flushed. - * - * => INVARIANT_1 violated. - * => this case needs a cache flush at onAfterCatchUp; to ensure the system converges. - * - * CASE B (cache flushed on onAfterCatchUp only): - * - Let's assume the blocking has finished, and caches have not been flushed yet. - * - Then, during re-rendering, the old content is shown because the cache is still full - * - * => INVARIANT_2 violated. - * => this case needs a cache flush at onBeforeBatchCompleted. - * - * SUMMARY: We need to flush the cache at BOTH places to ensure non of the cases above happens. BUT (!) - * Flushing the cache is a very expensive operation, so we need to reduce them as much as we can. - * So we decided to take the risk of CASE B, which might show an outdated content a bit longer than - * it might be expected. + * projection update + * call finished + * EventStore::commit + * ║ │ + * ─────╬──────────────────────────!1!───────────┼────────!2!─▶ + * ║ ▲│ + * │ │ + * │ │ NO async boundary anymore! + * │ │ => we can GUARANTEE that + * │ │ onAfterCatchUp has run + * │ │ SYNC before control is returned + * │ │ POINT to the caller. + * │ ║ │ + * Projection::catchUp │ │ ║ ││ + * ────────────────┼────┼──────────────╬───────┼──────────────▶ + * │ │ ║ │ + * update Projection │ ║ │ + * state (old -> new) │ ║ │ + * │ TX commit │ + * update sequence (end of batch)│ + * number │ + * │ + * onAfterCatchUp + * => e.g. flush + * Fusion cache * * @internal */