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

Error saving action: ActionScheduler_Action::$args too long #855

Open
wants to merge 7 commits into
base: trunk
Choose a base branch
from

Conversation

saad-siddique
Copy link

Closes #851

PHP Fatal error: Uncaught RuntimeException: Error saving action: ActionScheduler_Action::$args too long

Closes woocommerce#851

PHP Fatal error: Uncaught RuntimeException: Error saving action: ActionScheduler_Action::$args too long
Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together, @saad-siddique.

  • In a related conversation, there was discussion of using a hash to keep performance snappy: I think there is a lot of merit in adding that (but let me know if you considered this and found it was unnecessary or unhelpful).
  • Even if we move ahead without hashing, we would need various other changes. For example, this change would represent a new schema version and the change to existing method update_schema_5_0 won't be "seen" by installations where the schema is already up-to-date.

@barryhughes barryhughes added the needs feedback The issue/PR needs a response from any of the parties involved in the issue. label Sep 20, 2022
@saad-siddique
Copy link
Author

Hi @barryhughes ,

Thank you for taking the time to look at the issue.

Thank you for suggesting the hash method. It's possible to use that method, but it seems like a band-aid solution. The solution does work, however.

My main reason not to use the hash method is that instead of storing the Action Scheduler's related $args into its table, it's forcing us to store it in the postmeta or options table. It will pollute the tables meant for posts or options/settings. Another reason is the cleanup of past/failed/successful scheduled actions isn't going to drop related records from these tables in case the action failed to complete (like the action threw a fatal error during the process). At least deleting a record from AS tables will remove that data too.

Based on your comment above, I've updated the schema to 7 and added all the necessary code. The $max_args_length validation still exists but increased to store approx. half the length of the longtext field, which I believe should be sufficient enough for several cases, including at scale sites.

Please let me know if anything else needs to be updated.

@barryhughes
Copy link
Member

My main reason not to use the hash method is that instead of storing the Action Scheduler's related $args into its table, it's forcing us to store it in the postmeta or options table.

Yep, I agree that wouldn't be ideal.

Though, my reading of the conversation ("you could keep args as json encoded/serialized in a new meta field") was not that we would necessarily depend on core WordPress tables to store the full length args (which, of course, is currently possible even without this change).

In any case, I wanted to let you know we haven't lost sight of this change—but there are a few other items we're prioritizing right now, so there might be a further delay before we can really tackle this one.

@xdividr
Copy link

xdividr commented Aug 16, 2023

Following up to add 2¢ that I for one would appreciate seeing a robust solution for longer stored values to improve performance and stability relative to the hash method mentioned above. Thanks to all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs feedback The issue/PR needs a response from any of the parties involved in the issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP Fatal error: Uncaught RuntimeException: Error saving action: ActionScheduler_Action::$args too long
3 participants