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

Cache Warmup als Cronjob #1679

Open
hansmorb opened this issue Dec 1, 2024 · 5 comments
Open

Cache Warmup als Cronjob #1679

hansmorb opened this issue Dec 1, 2024 · 5 comments
Labels

Comments

@hansmorb
Copy link
Contributor

hansmorb commented Dec 1, 2024

Bei Instanzen, die viele Ansichten haben, kann es bei Aufruf eines Shortcodes zu langen Wartezeiten kommen, wenn zwischen diesem und dem letzten Aufruf zu viel Zeit vergangen ist. Wenn diese selten aufgerufenen Shortcodes auch noch viele Zeitrahmen abbilden, kann es für die Nutzenden zu längeren Wartezeiten kommen (Mehr dazu). Ein regelmäßig laufender Warmup Job könnte da Abhilfe schaffen. Dieser würde nach einem vordefinierten Intervall alle Shortcodes einmal ausführen und jede Kartenansicht laden. Damit das Sinn ergibt, muss jedoch WP-Cron in den System Task Scheduler eingebunden werden . Falls das nicht gemacht werden würde, wird WP-Cron nur bei Seitenaufruf gestartet. Damit wäre wahrscheinlich nicht viel gewonnen. Der Cronjob müsste dann auch mindestens alle 15 Minuten aufgerufen werden, damit die restlichen Jobs wie Reminder und Cleanup weiterhin funktionieren.

Vorteile:

  • Schnellere Cache Hits bei selten aufgerufenen Shortcodes
  • Schnellerer Seitenaufbau bei Tageswechsel
  • Einfach zu implementieren

Nachteile:

  • Konstante Ressourcenbelegung durch nachladende Shortcodes
  • WP-Cron als Systemtask zu konfigurieren ist nicht trivial
  • Nicht möglich, wenn nur Webspace gemietet ist
  • Leicht höhere Speicherbelegung durch größeren Cache

Risiken:

  • Timeout Probleme beim Ausführen des Jobs
  • Verschlechterung der Performance, wenn WP-Cron nicht als Systemtask konfiguriert ist
  • Konfiguration sehr individuell: Bei geringer Leistung führt häufiger Refresh Intervall eher zu Performanceschwierigkeiten. Bei Instanz mit vielen Zeitrahmen und häufigen Buchungen ist nächtlicher Warmup wahrscheinlich nicht ausreichend.

Falls wir das implementieren würde ich vorschlagen es als standardmäßig deaktivierte Option unter "Erweiterte Optionen" bei den Cacheeinstellungen zu verstecken. Dazu würde ich auch noch einen Hinweis auf die Einbindung von WP-Cron in den System Task Scheduler als Voraussetzung geben. Wir könnten auch die Konstanten DISABLE_WP_CRON als Indikator dafür nehmen, ob der System Task Scheduler aktiv ist.

@datengraben
Copy link
Contributor

Finde die vorgeschlagene Lösung eigentlich gut, da die Performance-Probleme wahrscheinlich eher bei den wenigen Instanzen auftaucht, welche sowieso Ressourcen z.B.für die Konfiguration des System Task-Schedulers verfügbar haben.

Finde es aber aus folgenden Gründen auch nicht gut: da es andererseits nur eine Lösung für Instanzen darstelle, welche nicht nur Webspace gemietet haben (sind wahrscheinlich nicht wenige), die den Cache konfiguriert haben und die das entsprechende Know-How besitzen. Und schlussendlich denke ich, dass es sich weiterhin nur um das konzentrieren auf Symptome des Grundproblems handelt. Also das Grundproblem des Zeitrahmen-Designs.1

Es wäre also vielleicht eine Abwägung von

  1. Wie viel des o.g. Grundproblems (siehe 1) können wir mit dieser Lösung (Cache Warmup als Cronjob) überwinden
  2. Für wen ist die Lösung dann relevant und ist diese Lösung am Ende auch einfacher zu handhaben (als Entwickler UND als User)

Denn eine Änderung des Zeitrahmen-Design ist zwar mehr Aufwand, aber gar nicht so komplex und am Ende auch für mehr User des Plugins relevant. Sie könnte z.B. wie folgt aussehen (die Vorschläge sind alternativ zu sehen und sortiert von viel Aufwand zu weniger Aufwand):

  • (A) Eigene Tabelle für das Zeitrahmen Objekt. Wir entfernen den Custom-post-Type Zeitrahmen und auch die Möglichkeit der Meta-Felder, Abfragen geschehen nur über die neue Tabelle.
    • Nutzung der WP Meta-Felder Schnittstelle nicht mehr möglich.
  • (B) Eigene Tabelle für die essentiellen Attribute, die einen Zeitrahmen definieren (start-date/end-date usw.). Wir behalten den Custom-Post-Type Zeitrahmen und die Meta-Felder und verbinden ihn nur mit Einträgen in der neuen Tabelle.
    • Nutzung der WP Meta-Felder weiterhin möglich.
  • (C) Einführen eines eigenen Post-Status (z.B. archived oder inactive) für alte Zeitrahmen um mögliche Indizierungen auf der wp_post-Tabelle (speziell dem post_status Attribut) nutzen zu können.
    • Wir könnten z.B. alle Zeitrahmen deren end-date vorrüber ist als inaktiv/archiviert markieren und diese bei der Berechnung der Verfügbarkeit nicht mehr beachten.

Der letzte Punkt (C) wäre natürlich sehr unaufwändig durch wenige Code-Zeilen umzusetzen, aber nach wie vor auch nur Symptom-Bekämpfung. Ich bin mir aber noch unsicher wie groß die Performance-Gewinne hier am Ende wären. Bin noch nicht dazu gekommen es zu testen.

Footnotes

  1. Grundproblem im Detail hier Commonsbooking Diskussion : Zeitrahmen sind als CustomPostType mit Attributen über die wp_postmeta Tabelle definiert, und so können die kritischen Datums/Zeit-Werte nicht performant abgefragt werden. 2

@hansmorb
Copy link
Contributor Author

hansmorb commented Dec 4, 2024

Finde die vorgeschlagene Lösung eigentlich gut, da die Performance-Probleme wahrscheinlich eher bei den wenigen Instanzen auftaucht, welche sowieso Ressourcen z.B.für die Konfiguration des System Task-Schedulers verfügbar haben.

Finde es aber aus folgenden Gründen auch nicht gut: da es andererseits nur eine Lösung für Instanzen darstelle, welche nicht nur Webspace gemietet haben (sind wahrscheinlich nicht wenige), die den Cache konfiguriert haben und die das entsprechende Know-How besitzen. Und schlussendlich denke ich, dass es sich weiterhin nur um das konzentrieren auf Symptome des Grundproblems handelt. Also das Grundproblem des Zeitrahmen-Designs.1

Ich stimme dir grundsätzlich zu. Leider habe ich schon einiges durchprobiert und das wäre eine sehr einfache Lösung. Vor allem hat die Flotte viele Karten, die auch etwas spezieller sind. Wenn die regelmäßig aufgewärmt werden könnten die auch flotter aufgerufen werden.

Es wäre also vielleicht eine Abwägung von

1. Wie viel des o.g. Grundproblems (siehe [1](#user-content-fn-site1-97cddf574d8612158616b8017b10c2e5)) können wir mit dieser Lösung (Cache Warmup als Cronjob) überwinden

Bei meinen Tests hat es einen großen Unterschied gemacht ob der Cache aufgewärmt ist oder nicht. Daher kam die Idee.

2. Für wen ist die Lösung dann relevant und ist diese Lösung am Ende auch einfacher zu handhaben (als Entwickler UND als User)

Wenn es einmal eingerichtet ist, dann ist das set it and forget it.

Denn eine Änderung des Zeitrahmen-Design ist zwar mehr Aufwand, aber gar nicht so komplex und am Ende auch für mehr User des Plugins relevant. Sie könnte z.B. wie folgt aussehen (die Vorschläge sind alternativ zu sehen und sortiert von viel Aufwand zu weniger Aufwand):

* **(A)** Eigene Tabelle für das Zeitrahmen Objekt. Wir entfernen den Custom-post-Type Zeitrahmen und  auch die Möglichkeit der Meta-Felder, Abfragen geschehen nur über die neue Tabelle.
  
  * Nutzung der WP Meta-Felder Schnittstelle nicht mehr möglich.

Das Problem hierbei ist, dass Zeitrahmen viele Metafelder hat und es dann auch ein großer Aufwand wäre neue Spalten einzurichten.

* **(B)** Eigene Tabelle für die essentiellen Attribute, die einen Zeitrahmen definieren (start-date/end-date usw.). Wir behalten den Custom-Post-Type Zeitrahmen und die Meta-Felder und verbinden ihn nur mit Einträgen in der neuen Tabelle.

Das wurde hier schonmal probiert: #1575 Der Performancegewinn war aber leider nur minimal, der Aufwand erheblich und es traten an vielen Stellen unerwartete Probleme auf.

  * Nutzung der WP Meta-Felder weiterhin möglich.

* **(C)** Einführen eines eigenen Post-Status (z.B. archived oder inactive) für alte Zeitrahmen um mögliche Indizierungen auf der `wp_post`-Tabelle (speziell dem `post_status` Attribut) nutzen zu können.

Die Idee finde ich gut und das wäre auch neu. Vielleicht als regelmäßiger Job, der Buchungen die älter als ein Monat sind archiviert?

  * Wir könnten z.B. alle Zeitrahmen deren `end-date` vorrüber ist als inaktiv/archiviert markieren und diese bei der Berechnung der Verfügbarkeit nicht mehr beachten.

Der letzte Punkt (C) wäre natürlich sehr unaufwändig durch wenige Code-Zeilen umzusetzen, aber nach wie vor auch nur Symptom-Bekämpfung. Ich bin mir aber noch unsicher wie groß die Performance-Gewinne hier am Ende wären. Bin noch nicht dazu gekommen es zu testen.

Footnotes

1. Grundproblem im Detail hier [Commonsbooking Diskussion](https://github.com/wielebenwir/commonsbooking/discussions/1465) : Zeitrahmen sind als CustomPostType mit Attributen über die `wp_postmeta` Tabelle definiert, und so können die kritischen Datums/Zeit-Werte nicht performant abgefragt werden. [↩](#user-content-fnref-site1-97cddf574d8612158616b8017b10c2e5) [↩2](#user-content-fnref-site1-2-97cddf574d8612158616b8017b10c2e5)

Ich habe auch mal probiert diese Queries zu optimieren, das ist leider auch fehlgeschlagen: #1583

@datengraben
Copy link
Contributor

@hansmorb Spannend! Danke für die ganzen Infos. Ich antworte nur mal auf die Punkte, die für mich noch Fragen aufwerfen.

Das wurde hier schonmal probiert: #1575 Der Performancegewinn war aber leider nur minimal, der Aufwand erheblich und es traten an vielen Stellen unerwartete Probleme auf.

Das habe ich total vergessen, aber erinnere mich daran, habe es damals gar nicht so eng verfolgt.

Ich habe mir den Branch angeschaut und frage mich wieso du die beiden DATETIME-Spalten der tfrelations-Tabelle nicht indiziert hast. Also entweder als Teil des Primary index oder als secondary index. Zumindest sehe ich diese nicht in dem Stand des Codes. Das diese Spalten Teil der WHERE-Clause einer SQL-Anfrage sind, ist ja fast schon die Regel, sie zu indizieren also eigentlich notwendig.

Die Idee finde ich gut und das wäre auch neu. Vielleicht als regelmäßiger Job, der Buchungen die älter als ein Monat sind archiviert?

Ja genau. Das lässt sich eigentlich auch sehr schnell auf einer Umgebung mit genügend Timeframes testen. Ggf. wäre die Nutzung des System Scheduler dafür dann gar nicht nötig. Aber wenn du eh schon weißt wie es geht, ist das ja ggf. gar nicht mehr so ein Akt. Ich würde mir dessen Nutzung auch wünschen.

Einziger Punkt gegen den post_status: Ich hatte nur in Erinnerung das die Query-Logik in Wordpress oder CommonsBooking (weiß leider nicht mehr wo das im Code auftaucht) einen Array von Post-IDs (teilweise mehrere dutzend oder >100) mitgibt, welche dann schon auf die wp_posts-Tabelle filtern. Kann also sein das die Gewinne gar nicht so groß wären, aber ausprobieren würde ich es auf jeden Fall. Falls die Spalte post_status nicht indiziert ist könnte man das auch nachholen.

@hansmorb
Copy link
Contributor Author

@hansmorb Spannend! Danke für die ganzen Infos. Ich antworte nur mal auf die Punkte, die für mich noch Fragen aufwerfen.

Das wurde hier schonmal probiert: #1575 Der Performancegewinn war aber leider nur minimal, der Aufwand erheblich und es traten an vielen Stellen unerwartete Probleme auf.

Das habe ich total vergessen, aber erinnere mich daran, habe es damals gar nicht so eng verfolgt.

Ich habe mir den Branch angeschaut und frage mich wieso du die beiden DATETIME-Spalten der tfrelations-Tabelle nicht indiziert hast. Also entweder als Teil des Primary index oder als secondary index. Zumindest sehe ich diese nicht in dem Stand des Codes. Das diese Spalten Teil der WHERE-Clause einer SQL-Anfrage sind, ist ja fast schon die Regel, sie zu indizieren also eigentlich notwendig.

Guter Einwand, hätte ich wohl mal besser in meiner Datenbank-Vorlesung aufgepasst ;) Ja vielleicht bringt das ja den entscheidenden Performanceschub. Ich habe nur glaube ich in nächster Zeit keinen Kopf mehr mich da nochmal intensiv reinzulesen und das anzupassen sowie da die Fehler auszumerzen.

Die Idee finde ich gut und das wäre auch neu. Vielleicht als regelmäßiger Job, der Buchungen die älter als ein Monat sind archiviert?

Ja genau. Das lässt sich eigentlich auch sehr schnell auf einer Umgebung mit genügend Timeframes testen. Ggf. wäre die Nutzung des System Scheduler dafür dann gar nicht nötig. Aber wenn du eh schon weißt wie es geht, ist das ja ggf. gar nicht mehr so ein Akt. Ich würde mir dessen Nutzung auch wünschen.

Welche Nutzung würdest du dir wünschen? Also der Cronjob so wie er oben beschrieben ist oder das System mit dem post_status?

Einziger Punkt gegen den post_status: Ich hatte nur in Erinnerung das die Query-Logik in Wordpress oder CommonsBooking (weiß leider nicht mehr wo das im Code auftaucht) einen Array von Post-IDs (teilweise mehrere dutzend oder >100) mitgibt, welche dann schon auf die wp_posts-Tabelle filtern. Kann also sein das die Gewinne gar nicht so groß wären, aber ausprobieren würde ich es auf jeden Fall. Falls die Spalte post_status nicht indiziert ist könnte man das auch nachholen.

Stimmt! Die getPostIdsByType kümmert sich nicht um den post_status. Und da die Rückgabe dieser Funktion dann von der getPostsByBaseParam gefiltert wird wäre da vermutlich nichts zu gewinnen.

@datengraben
Copy link
Contributor

Guter Einwand, hätte ich wohl mal besser in meiner Datenbank-Vorlesung aufgepasst ;) Ja vielleicht bringt das ja den entscheidenden Performanceschub. Ich habe nur glaube ich in nächster Zeit keinen Kopf mehr mich da nochmal intensiv reinzulesen und das anzupassen sowie da die Fehler auszumerzen.

Kann ich verstehen, die Entwicklung des ganzen Branchs fertigzustellen ist natürlich ein andere Punkt. Ist der Aufwand nur zum Testen des Index-Setups im bestehenden Branch vertretbar? Also könnte ich den Branch auschecken, den Index verändern und es direkt testen oder kommt etwas mehr Arbeit auf mich zu? Habe jetzt nicht gesehen ob ich es per Hand migrieren muss etc.

Ja genau. Das lässt sich eigentlich auch sehr schnell auf einer Umgebung mit genügend Timeframes testen. Ggf. wäre die Nutzung des System Scheduler dafür dann gar nicht nötig. Aber wenn du eh schon weißt wie es geht, ist das ja ggf. gar nicht mehr so ein Akt. Ich würde mir dessen Nutzung auch wünschen.

Welche Nutzung würdest du dir wünschen? Also der Cronjob so wie er oben beschrieben ist oder das System mit dem post_status?

Ne den Cronjob. Wir haben z.B. manchmal weniger Traffic, d.h. der normale Scheduler läuft dann auch nur wenn mal jemand auf die Seite kommt.

Einziger Punkt gegen den post_status: Ich hatte nur in Erinnerung das die Query-Logik in Wordpress oder CommonsBooking (weiß leider nicht mehr wo das im Code auftaucht) einen Array von Post-IDs (teilweise mehrere dutzend oder >100) mitgibt, welche dann schon auf die wp_posts-Tabelle filtern. Kann also sein das die Gewinne gar nicht so groß wären, aber ausprobieren würde ich es auf jeden Fall. Falls die Spalte post_status nicht indiziert ist könnte man das auch nachholen.

Stimmt! Die getPostIdsByType kümmert sich nicht um den post_status. Und da die Rückgabe dieser Funktion dann von der getPostsByBaseParam gefiltert wird wäre da vermutlich nichts zu gewinnen.

Ja das meinte ich! Da müssten wir vermute ich nochmal ran, vielleicht gibt es einen besseren Weg. Also nicht erst ausschließlich nur auf die postmeta Tabelle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

2 participants