I am scheduling node transitions on minute granularity.
But it does not work efficiently when lightning_scheduler_cron() runs per minute.

A copy of https://www.drupal.org/node/3102027.

Comments

abhaysaraf created an issue. See original summary.

abhaysaraf’s picture

StatusFileSize
new250.66 KB

Patch added.
Also an extention to https://www.drupal.org/node/3068420.

abhaysaraf’s picture

Version: 8.x-1.1 » 8.x-3.10
abhaysaraf’s picture

StatusFileSize
new250.66 KB

Updated patch name following convention https://www.drupal.org/patch/submit

phenaproxima’s picture

Project: Lightning Workflow » Lightning Scheduler
Version: 8.x-3.10 » 8.x-1.x-dev
joseph.olstad’s picture

convert this into a patch:

diff --git a/js/TransitionSet.es6.js b/js/TransitionSet.es6.js
index 2f12e07..3aad218 100644
--- a/js/TransitionSet.es6.js
+++ b/js/TransitionSet.es6.js
@@ -24,6 +24,13 @@ export default class extends Component
 
         this.state = {
             transitions: transitions.map(t => {
+                // Special case: when node edit page is viewed after preview.
+                // Unix timestamp is stored in input of Drupal.
+                // Converting this so that this is readable by JS.
+                if (typeof t.when == 'number') {
+                    t.when = t.when * 1000;
+                }
+
                 // The date and time of a transition is passed in as an ISO
                 // 8601 UTC string, which we need to convert to a date object.
                 t.when = new Date(t.when);
@@ -278,13 +285,19 @@ export default class extends Component
     toJSON ()
     {
         return this.state.transitions.map(t => {
+            // Trim additional seconds based on configured granularity.
+            let trimWhen = t.when.getTime() / 1000;
+            if (this.props.step >= 60) {
+                trimWhen -= trimWhen % 60;
+            }
+
             return {
               // JavaScript returns time stamps in millseconds, but PHP handles them
               // in seconds. Divide by 1000 to convert to seconds, then round down to
               // ensure we do not send a floating-point value back to the server. We
               // always round down (instead of using Math.round()) to in order to
               // prevent off-by-one-second timing failures in tests.
-              when: Math.floor(t.when.getTime() / 1000),
+              when: Math.floor(trimWhen),
               state: t.state,
             };
         });

and also, this to speed up cron jobs, previously my cron job was taking 4 to 5 minutes because the cron was reprocessing all records since the beginning of unix time (Jan 1, 1970), every cron run. Really we only have to go back as far as the last cron run but this logic goes back 8 weeks , to improve it, get the timestamp of the last cron run, and make it run a couple hours before that or a day before just to make sure everything is processed and nothing is missed based on time zone differences so avoid timezone issues, maybe two days in front of that for reprocessing just to be safe. However going back to Jan 1 1970 every cron run is a bit too much to ask of a server.

diff --git a/src/TransitionManager.php b/src/TransitionManager.php
index 1a17271..0847387 100644
--- a/src/TransitionManager.php
+++ b/src/TransitionManager.php
@@ -234,14 +234,22 @@ final class TransitionManager {
   private function getTransitionable($entity_type_id, DrupalDateTime $now) {
     $storage = $this->entityTypeManager->getStorage($entity_type_id);
     $languages = \Drupal::languageManager()->getLanguages();
-    $now = $now->format(DateTimeItemInterface::DATETIME_STORAGE_FORMAT);
-
+    $sql_timezone = DateTimeItemInterface::STORAGE_TIMEZONE;
+    $storage_format = DateTimeItemInterface::DATETIME_STORAGE_FORMAT;
+    $now = $now->format($storage_format, ['timezone' => $sql_timezone]);
+    $weeks_ago = new DrupalDateTime('+56 days ago', $sql_timezone);
+    $weeks_ago = $weeks_ago->format($storage_format, ['timezone' => $sql_timezone]);
     // Entities are transitionable if its latest revision has any transitions
-    // scheduled now or in the past.
+    // scheduled now or in the past going back 56 days ago (8 weeks).
+    // We should not reprocess going back to 1970 beginning of unix time.
+    // Limit past re-processing for performance and scalability reasons.
+    // The only reason why to go back weeks is in case cron hasn't run.
+    // To improve this, only go back in time as far as the last successful cron.
     $IDs = $storage->getQuery()
       ->latestRevision()
       ->accessCheck(FALSE)
       ->condition('scheduled_transition_date.value', $now, '<=')
+      ->condition('scheduled_transition_date.value', $weeks_ago, '>=')
       ->execute();
 
     foreach ($IDs as $revision_id => $entity_id) {
joseph.olstad’s picture

Status: Active » Needs review

roll up a patch based on my comment above, set it for review

joseph.olstad’s picture

joseph.olstad’s picture

StatusFileSize
new3.29 KB

patch for review

joseph.olstad’s picture

StatusFileSize
new3.37 KB

new patch
no need to go weeks back.

joseph.olstad’s picture

Issue tags: -Drupal 8 +Performance, +scalability
joseph.olstad’s picture

Priority: Normal » Critical

Due to the severity of this performance issue (going back and reprocessing all nodes modified since the beginning of unix time (jan 1 1970) over and over again) I am increasing this to a critical.

This module is not long-term usable without this patch unless you only have a very small amount of content.

Status: Needs review » Needs work

The last submitted patch, 10: lightning_scheduler-3102295-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

joseph.olstad’s picture

hmm, comparing 9 and 10 test results.

worst case scenario, go with patch 9.

kaszarobert’s picture

Status: Needs work » Closed (duplicate)

The issue's original problem was fixed in #3102027: Transition does not happen accurately when lightning_scheduler_cron() runs each minute
The performance issue mentioned later will be dealt with in #3379796: Performance - Prevent redundant processing back to January 1st 1970 on every cron run.

So this issue duplicates both, to eliminate confusion I'm closing this in favor of the separate issues.