Performance - Prevent redundant processing back to January 1st 1970 on every cron run

Created on 7 August 2023, over 1 year ago
Updated 17 August 2024, 3 months ago

Problem/Motivation

Bumping this to critical.

The solution is outlined quite some time ago in comment #3003907-73: Make this work with multilingual β†’

Without this solution your site can become completely hampered by performance issues caused by redundant processing going back to January 1st 1970.

Steps to reproduce

Start using lightining_scheduler, schedule several transitions over a period of several months/years.
Notice your site getting slower and slower the more that scheduler transitions accumulate.

Proposed resolution

See patch

Remaining tasks

Review and commit the patch and tag a release.

User interface changes

N/A

API changes

Goes back to last cron run minus a couple days (no more) to ensure everything is processed but not excessively redundantly. Each processing loads the node into memory, can become extremely onerous over time and the logic makes no sense and can lead to excessive denials of service.

The end result is a denial of service and overtaxing of servers.

Data model changes

N/A

πŸ› Bug report
Status

Fixed

Version

1.0

Component

Code

Created by

πŸ‡¨πŸ‡¦Canada joseph.olstad

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @joseph.olstad
  • Status changed to Needs review over 1 year ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 7.3 & PostgreSQL 10.12
    last update over 1 year ago
    33 pass
  • πŸ‡¨πŸ‡¦Canada joseph.olstad

    This module has a very nice UI and works well for monolingual implementations, however underneath the prettiness lies a very nasty issue as described in the issue summary that this patch will resolve.

    Bumping this to Critical since it leads to increasing vulnerability to denial of service, also taxes servers unnecessarily and can lead to severe performance issues. Most people who experience this probably think that the problem is Drupal it'self, very hard to diagnose and troubleshoot especially for most people.

    We should just fix the glitch to help others that may be still using this module.

  • πŸ‡¨πŸ‡¦Canada joseph.olstad

    still critical, until this is fixed, use the "Scheduled Transitions" module instead

  • First commit to issue fork.
  • Status changed to RTBC 3 months ago
  • πŸ‡¨πŸ‡¦Canada joseph.olstad

    Aside from lacking the dependency injection for this: \Drupal::state()->get('system.cron_last'); near parfect. The MR11 is satisfactory for me as-is.

  • πŸ‡¨πŸ‡¦Canada joseph.olstad

    please put a checkbox next to our names at the bottom of this issue and press save

  • πŸ‡ΈπŸ‡°Slovakia kaszarobert

    @joseph.olstad I created the MR with the exact same changes you uploaded in a patch years ago. However, I had to change a few things:
    - I fixed the proper DI in the class,
    - With this change, we're now disabling the processing of older transitions than last cron run, I added a new test case for that
    - During writing a test for this new behavior, I found out that in the patch you were using a timestamp in the SQL query for the lowest scheduled_transition_date.value but in the database that field contains a Y-m-d\TH:i:s formatted UTC timezoned date, so the patch by default didn't change the requested behavior the way we wanted it, so I modified the code to pass a formatted date to the query.
    - this a backwards breaking change, so I'm thinking about if we should set this new 'lowest date of a state transition' thing into a setting instead. But again due to how this module was architected + with the performance issues you were experiencing indicate that this should be the default behavior to not break huge sites. So maybe we have no other choice than commit this really.

  • Status changed to Fixed 3 months ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024