2.8.x: Database::execute() vs. Database::executeUncached()

[deerwood]

Moin alle,

kann jemand bitte näher erläutern, was in TL 2.8.x zu beachten ist, wenn man in einer Erweiterung Database::execute() (die nun das Resultat cached) benutzt?

Ich habe gesehen, dass in einigen Erweiterungen von Database::execute() auf Database::executeUncached() umgestellt worden ist. Was waren Eure Gründe dafür?

Anlass: ich habe gerade ein Problem im Umfrage Tool [survey_ce] (hoffentlich wirklich) fixen können, das in 2.7.x nicht existierte, aber in 2.8.x auftaucht. Der Original-Code ist zwar nicht optimiert, sieht für mich aber auch nicht falsch aus.

In 2 verschiedenen Methoden ein und der selben Klasse, die sehr verschiedene Ergebnisse liefern, wurde das gleiche Query mit den gleichen Parametern benutzt. Die 1. Methode rief, in einer while Schleife über die Resultate, die 2. Methode auf. Folge: in der 1. Methode lieferte die while Schleife nur noch den ersten Datensatz.

Ich weiss, das der Original-Code suboptimal ist und habe ja auch eine Lösung (und auch diverse Workarounds, siehe hier).

Ich befürchte halt, dass sich in dem neuen 2.8.x Feature des "silent" Cachings noch ein subtiler Fehler befindet, der möglicherweise viele Erweiterungen betreffen könnte.

Eure Meinung, Erfahrung, Erkenntnis? LG, Georg

[xtra]

Grundsaetzlich gilt, jedesmal wenn du Daten aus der Datenbank holst, modifizierst und anschliessend wieder holen willst (egal ob im selben Modul oder von woanders aus), dann musst du anschliessend die uncached verwenden oder gar beide Male uncached arbeiten (sicherer).

Beispiel (ohne Check und Sinn und Zweck):
Modul A:

$objDemo=$this->Database->prepare('SELECT * FROM tl_blafasel WHERE stumpfsinn=1;')
              ->execute();
if($objDemo->numRows)
{
  $this->Database->prepare('UPDATE tl_blafasel SET stumpfsinn=? WHERE id=?')
                 ->execute(0, $objDemo->id);
} 

Modul B (bzw. eine Routine spaeter im Workflow):

$objDemo=$this->Database
              ->prepare('SELECT * FROM tl_blafasel WHERE stumpfsinn=1;')
              ->execute(); 

Preisfrage, was findet das zweite Query nun?
Ist da die Zeile von oben enthalten oder nicht?
Hat sie noch Stumpfsinn=1?
Antwort: logisch ist sie enthalten, denn das Ergebnis ist gecached. ebenso "denkt" sie auch, dass sie noch stumpfsinnig ist.

Ebenso verhaelt es sich mit Daten, die du in die Datenbank einfygst (die sind dann nicht im Cache und existieren somit fyr die Datenbankklasse noch gar nicht), und nicht nur mit Daten die du aenderst.
Und ganz schlimm wird es, wenn du "mittendrin" loeschst, ohne dass der Cache es mitbekommt, da findest du dann die tollsten Sachen mit "haufenweise" toten IDs.

Kurzum: Wenn du Datenschrubben willst (UPDATE, INSERT, DELETE), dann hole sie vorher uncached.
Wenn du nur Datenschaufeln willst (SELECT und dann ab ins Template), dann solltest du cached arbeiten koennen (haengt aber wiederum vom einzelnen Anwendungsfall ab).
Kurzum es gibt kein "richtig" oder "falsch" oder "wie macht ihr das", es gibt nur ein "wie muss ich es hier machen".

hth
Chris

[deerwood]

Moin Chris,

jupp, das ...->isModified() habe ich gesehen.

Im geschilderten Fall sind es aber nur 2 SELECT Statements und ich bin sicher, dass keine der beiden Methoden die DB bzw. das Resultat aktiv verändert.

Offenbar ist es so (soweit ich es bisher verstanden habe), dass der Aufruf der 2. Methode aus der 1. Methode den Result-Pointer hinter/ans Ende des Resultats bewegt. Deshalb ist plötzlich und unerwartet die while-Schleife in der 1. Methode am Ende.

LG, Georg

[xtra]

Kannst du den Call-Stack evtl. hier kurz charakterisieren?
Handelt es sich bei beiden Aufrufen um dasselbe Objekt, jedoch innerhalb einer Schleife?
In etwa so?

  function_1()
  {
     $objDate=... SELECT ...
     while($objData->next)
     {
        function_2();
        // bricht hier dann ab?
     }
  }

  function_2()
  {
     $objDate=... SELECT ...
     while($objData->next)
     {
        // loopt durch Daten
     }
  }

Das sollte eigentlich nicht sein, denn die Datenbank checkt, wie du schon richtig festgestellt hast, mit isModified ob das ResultSet selbst veraendert wurde. Sollte dem so sein, so wird es neu aus der DB geholt (Dein altes existiert dann logischerweise nicht mehr im Cache).
Sollte das ResultSet jedoch unmodifiziert sein, so wird dir das gecachte Object (nachdem es resetted wurde/Pointer auf Anfang) zurueckgegeben.

Ich verstehe nicht genau wo dein Problem nun liegt.
Waere schoen wenn du es in einem Audruf diagramm oder aehnlichem darlegen koenntest oder aber zumindest die beiden betroffenen Codestellen.

[acenes]

So wie du den Ablauf beschreibst funktioniert das tatsächlich nicht mehr mit execute(). Das liegt aber nicht an einem "subtilen Fehler" in der Implementation des gecachten execute() sondern an der Tatsache dass jetzt exakt dasselbe Query verschachtelt verwendet wird. Das innere Query verwendet neu den gleichen Pointer des Resultset wie das äussere, was dann natürlich auch wieder auf die äussere Schleife zurückwirkt.

Man kann sich die Wirkung so vorstellen:

for ($pointer = 0; $pointer < $anzahlRecords; $pointer++) {
    // innere Schleife
    for ($pointer = 0; $pointer < $anzahlRecords; $pointer++) ....
    // $pointer ist jetzt == $anzahRecords
}

In deinem Beispiel müsste man das äussere Query also mit executeUncached ausführen und das innere mit execute, dann gibt es 2 unabhängige Resultsets, und trotzdem profitiert die innere Schaufe vom caching.

Bei neuen Erweiterungen würde ich execute fallweise für SELECT verwenden, und für alles andere (UPDATE, INSERT, DELETE etc) grundsätzlich executeUncached weil es schneller ist.

[deerwood]

Hi Chris, acenes,

sorry für die Verzögerung, ich musste ja auch mal schlafen und dann Brot erwerben.

Eure beiden Skizzen des Code-Ablaufs sind korrekt, genau so verhielt es sich. Die Schleife brach genau dort ab, wo Chris das markiert hat (lieferte nur den 1. Datensatz von vielen) und acenes Beispiel zeigt, warum das zu dem Fehler führte.

Das Problem, das ich nach wie vor sehe, ist: der Code war ja RICHTIG!!! Und würde man eine Kopie des Resultsets statt einer Referenz auf den Resultset bekommen, dann würde es keine Probleme geben. Die DB würde dann auch nur 1 mal bemüht, aber das Iterieren über die Datensätze wäre unabhängig voneinander (auf Kosten von Memory).

Ich könnte mir vorstellen, dass einige Erweiterungen, die für 2.7.x geschrieben wurden, ähnliche Probleme haben könnten. Ist ja oft so, dass man ein Query mit Copy/Paste hernimmt, weil es in etwa passt bzw. sowieso ein "SELECT *" hat.

So wie es ist, müssten Extension-Programmierer vor jedem Hinschreiben eines SQL-Statements erstmal prüfen, ob es ein identisches Statement (mit potentiell den gleichen Parametern) nicht schon irgendwo gibt. Und das nicht nur in der eigenen Erweiterung, sondern auch im ganzen Core und eigentlich auch in sämtlichen existierenden Extensions. Und dann beurteilen, ob das zum Problem werden kann.

Oder eben immer executeUncached() benutzen, wodurch das neue Caching in execute() irgendwie obsolet wird, die Erweiterung dann auch nur noch 2.8.x kompatibel ist, es sei denn man baut auch noch einen Versions-Check ein bzw. prüft mit method_exists().

Meiner Meinung nach wäre es besser gewesen, execute() unverändert zu lassen und eine neue Methode executeCached() einzuführen. Dann hätten existierende Extensions solche Probleme nicht.

LG, Georg

[xtra]

Meiner Meinung nach wäre es besser gewesen, execute() unverändert zu lassen und eine neue Methode executeCached() einzuführen. Dann hätten existierende Extensions solche Probleme nicht.

Hierbei muss ich dir leider zustimmen, aber da gibt es nun keinen Weg zuryck. Das ist nun so wie es ist.

Deine Idee mit der Kopie ist IMO auch nicht so das gelbe vom Ei, da sie wie du bereits sagst zu Lasten des Speichers geht.
Der wird uns dann irgendwann ausgehen wenn wir sowas anfangen.

Generell frage ich mich jedoch, in welchen Faellen bringt der Cache aktuell eigentlich was? also welche (gleichen) Queries werden haeufig pro page load ausgefyhrt, so dass der Cache greifen kann?

Hierzu gilt es meiner Meinung nach einmal zu forschen.

[lindesbs]

Meiner Meinung nach wäre es besser gewesen, execute() unverändert zu lassen und eine neue Methode executeCached() einzuführen. Dann hätten existierende Extensions solche Probleme nicht.
LG, Georg

Jepp, da stimme ich Dir auch zu. Hatte Leo dazu vor ueber 2 Monaten mal einen BugBericht geschrieben : http://https://contao.org/issues/1344#change-4440
Ich sehe das auch als grosses Problem, aber Leo meinte daruf nur noch : Und nur weil Du einige Stunden gebraucht hast, um das Prinzip zu verstehen, ist es noch lange kein TYPOlight-Bug.

ALso merke Dir : Das ist kein Bug, du hast nur das Prinzip nicht verstanden.

[acenes]

Und würde man eine Kopie des Resultsets statt einer Referenz auf den Resultset bekommen, dann würde es keine Probleme geben. Die DB würde dann auch nur 1 mal bemüht, aber das Iterieren über die Datensätze wäre unabhängig voneinander (auf Kosten von Memory).

Die technisch beste Lösung wäre natürlich die Daten nur einmal zu speichern, jedoch die Pointer pro Query zu instanzieren. Ich fürchte allerdings dass der kompliziertere Code dann einen Grossteil des möglichen Gewinns der Cacherei wiederaufbrauchen könnte.

Ich könnte mir vorstellen, dass einige Erweiterungen, die für 2.7.x geschrieben wurden, ähnliche Probleme haben könnten. Ist ja oft so, dass man ein Query mit Copy/Paste hernimmt, weil es in etwa passt bzw. sowieso ein "SELECT *" hat.

Genau das sind die Fälle wo der Cache dann auch etwas bringt.

So wie es ist, müssten Extension-Programmierer vor jedem Hinschreiben eines SQL-Statements erstmal prüfen, ob es ein identisches Statement (mit potentiell den gleichen Parametern) nicht schon irgendwo gibt. Und das nicht nur in der eigenen Erweiterung, sondern auch im ganzen Core und eigentlich auch in sämtlichen existierenden Extensions

Eigentlich nur in der eigenen Erweiterung und im Core. Was andere Erweiterungen machen ist irrelevant, ausser deine Erweiterung baut darauf auf.

[deerwood]

Moin Chris,

Hierbei muss ich dir leider zustimmen, aber da gibt es nun keinen Weg zuryck. Das ist nun so wie es ist.

Na ja, nicht wirklich. Wäre ja leicht, in 2.8.2 execute() und executeUncached() zu Synonymen zu machen, damit bereits umgestellte Extensions sicher (wenn auch vielleicht weniger performant) weiter funktionieren. Und dann executeCached() einführen, die Extension-Entwickler darauf hinweisen und einmal durch den Core gehen (Fleissarbeit, oder?) und die SQL Statements, die das Caching wirklich risikolos verdienen, entsprechend umstellen.

Deine Idee mit der Kopie ist IMO auch nicht so das gelbe vom Ei, da sie wie du bereits sagst zu Lasten des Speichers geht.
Der wird uns dann irgendwann ausgehen wenn wir sowas anfangen.

Hmm, er würde kaum mehr ausgehen, als in 2.7.x, wenn ich mich nicht irre. Siehe aber auch acenes Idee mit nur der Kopie der Iteratoren, die ich im Prinzip gut finde.

Generell frage ich mich jedoch, in welchen Faellen bringt der Cache aktuell eigentlich was? also welche (gleichen) Queries werden haeufig pro page load ausgefyhrt, so dass der Cache greifen kann?

Hierzu gilt es meiner Meinung nach einmal zu forschen.

Ja, das denke ich auch. Und, natürlich dürfen die Queries auch nur sequentiell ausgeführt werden, sonst MUSS man ja executeUncached() an der einen oder anderen Stelle benutzen.

LG, Georg

[deerwood]

Moin lindesbs,

BugBericht ... http://https://contao.org/issues/1344#change-4440
Ich sehe das auch als grosses Problem,

Zu dem Ticket habe ich noch eine Frage: hattest Du das SELECT vor dem INSERT bereits einmal ausgeführt und nach dem INSERT wiederholt? Dann könnte ich beginnen zu verstehen. Siehe auch xtra's "Ebenso verhaelt es sich mit Daten, die du in die Datenbank einfygst".

Falls direkt nach einem INSERT ein zugehöriges SELECT nicht die vollständigen Daten abliefert (obwohl das in 2.7.x funktionierte), dann erscheint mir das neue execute() mehr als "subtil" fehlerhaft.

LG, Georg

[staen]

Ich finde das Caching ja an sich eine gute Idee...

Aber: "Silent Caching" finde ich in der Art der Implementation nicht wirklich sinnvoll. Warum?

Wenn ich in meinen Extensions mehrmals in einem Rutsch auf die gleichen Daten zugreife, dann speichere ich sie selber weg (als "static" in der Klasse) und achte bei Änderungen darauf, sie auch in meiner Variable auszuführen. So habe ich es gezielt in der Hand.

"Silent Caching" macht meiner Meinung dann Sinn, wenn ich eine höhere Datenbankabstraktion als gegeben habe. Wenn ich also z.B. statt selbst SQL-Statements zu schreiben in der Form

$this->Database->tl_table->field = $varValue 

schreibe und per

$varValue = $this->Database->tl_table->field 

lese. Dann kann ich nämlich schön parallel dazu den Cache pflegen.

Wenn ich dann noch dafür sorge, dass jedes "INSERT" oder "UPDATE" oder "DELETE" - falls ich doch "raw" SQL übergebe - den Cache leert (was ja evtl. sogar jetzt möglich sein sollte?) könnte es sogar klappen mit dem Caching.

Glaube ich.

Carsten