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

Saving UserForms with many fields causes timeouts #773

Open
Firesphere opened this issue May 16, 2018 · 21 comments
Open

Saving UserForms with many fields causes timeouts #773

Firesphere opened this issue May 16, 2018 · 21 comments

Comments

@Firesphere
Copy link

Firesphere commented May 16, 2018

Overview

More often than not, saving the form in the CMS times out from Apache/Nginx (30+ seconds)

Steps to replicate

  1. Create a form with 2 "pages" and at least 4 fields per page.
  2. Attempt to save, and it'll time-out. It's quite simple to replicate on a more lightweight server, but also on heavier, like SCP large systems, as soon as you hit, say 10 fields, the saving becomes unuseable.

Notes

Performance/Profiling Results

@dhensby
Copy link
Contributor

dhensby commented May 16, 2018

We had done some work on improving this, what version are you experiencing this on?

@dhensby
Copy link
Contributor

dhensby commented May 16, 2018

see #538

@Firesphere
Copy link
Author

Latest of all. Specifically probably worth mentioning, ElementalForms instead of just UserForms

@dhensby
Copy link
Contributor

dhensby commented May 16, 2018

Right, perhaps there's been some regression around this logic; otherwise, if it's just the sheer number of objects being saved, the only other choice is to move this into a queued job :/

@Firesphere
Copy link
Author

My first thought would be to move it to a raw query and then publish, instead of going down the rabbit hole of object instantiation?

@dhensby
Copy link
Contributor

dhensby commented May 16, 2018

I guess the first steps are really to profile where the time is being taken... then we decide from there

@Firesphere
Copy link
Author

My profiling efforts so far have not given single PoE, I have so far only found "it takes way too long", without any useful profiling stats. :(

@bergice
Copy link
Contributor

bergice commented Sep 21, 2020

Closing as we have no clear objective for moving forward with this. Happy to reopen if someone has any suggestions.

@bergice bergice closed this as completed Sep 21, 2020
@Firesphere
Copy link
Author

Firesphere commented Sep 22, 2020

Reopen please, as the userforms saving feature still takes more than 30 seconds, if not more, to process on multiple systems.

Also, if you don't have an objective moving forward.... then.... huh?

This was referenced Sep 22, 2020
@bergice
Copy link
Contributor

bergice commented Sep 25, 2020

@Firesphere I'll reopen once you've provided some clear examples with replication steps on how to make the requests time out.

@Firesphere
Copy link
Author

Create a form with 2 "pages" and at least 4 fields per page.

Attempt to save, and it'll time-out. It's quite simple to replicate on a more lightweight server, but also on heavier, like SCP large systems, as soon as you hit, say 10 fields, the saving becomes unuseable.

@bergice
Copy link
Contributor

bergice commented Sep 28, 2020

Create a form with 2 "pages" and at least 4 fields per page.

Attempt to save, and it'll time-out. It's quite simple to replicate on a more lightweight server, but also on heavier, like SCP large systems, as soon as you hit, say 10 fields, the saving becomes unuseable.

Thanks, I can confirm this does eventually result in a timeout if you add enough fields.

Profiling Results

I did some profiling where I made the following observations:

  • Every field is saved, and this is obviously slowing things down. Not sure how to fix this as it seems like a problem at the core of the ORM. But a good start would be to stop saving all fields unless they've changed. This could potentially be done in GridFieldEditableColumns::handleSave
  • The SiteTreeLinkTracking extension is slowing things down a lot
  • The FileLinkTracking extension is slowing things down a lot
  • The AssetControlExtension extension is slowing things down a lot

Performance Results

I also did some performance testing, by checking execution times after disabling the extensions that were causing trouble:

Here are the execution times for saving a UserForm with 2 pages and a total of 19 different fields.

Normal Execution Times

  • Save, No fields changed: 8 sec
  • Save, All field titles changed: 10 sec
  • Save, All field titles changed, All field types changed: 9 sec
  • Publish, All field titles changed: 13 sec

Optimised Execution Times

(FileLinkTracking, SiteTreeLinkTracking, AssetControlExtension extensions disabled)

  • Save, No fields changed: 3 sec
  • Save, All field titles changed: 3 sec
  • Save, All field titles changed, All field types changed: 4 sec
  • Publish, All field titles changed: 4 sec

@bergice bergice reopened this Sep 28, 2020
@bergice bergice changed the title Speed in CMS has room for improvement Saving UserForms with many fields causes timeouts Sep 28, 2020
@Firesphere
Copy link
Author

Here are the execution times for saving a UserForm with 2 pages and a total of 19 different fields.

Slightly better than the circumstances at the time I opened the issue, but still, in my opinion, far from acceptable.

Primary example can be provided via direct contact, for NDA reasons.

@elkebe
Copy link

elkebe commented May 14, 2021

UserDefinedForm will save, but won't publish with many fields (#1063)

When I create a UserDefinedForm page, I can save and publish it until I get to around 80-100 fields (yes a ridiculous number). After that, page will save, but not publish. Saving page results in each field being marked as unpublished. Form appears to take longer to save/publish for each form group that is added.

If it helps, am currently developing site on MacBook Pro 2012 using MAMP, SS4.7.3 + latest stable version of UserDefinedForms (not dev version).

@Firesphere
Copy link
Author

I can save and publish it until I get to around 80-100 fields

Actually not that ridiculous. Some clients (please contact me via email for details) have that amount of fields in their forms. However, at least half of them, if not more, are hidden conditional fields.

Especially for more complicated sign-up forms (e.g. telco's), this amount of fields in total would not surprise me.

@dhensby
Copy link
Contributor

dhensby commented May 14, 2021

I think having 80 - 100 fields is pushing the limits of the intention of this module. I'd suggest that if you're using that many fields then there's an argument that this needs to be implemented in a bespoke manner and not via a CMS administrated/built form.

There will always be capacity limitations to any system and timeouts under heavy workloads are inevitable if the load is simply too great. In this instance, that appears to be the problem.

Now I agree that if you can't get 10 fields working on a reasonable system, that's a problem making the module pretty useless for a fairly reasonable requirement. But if you're getting to triple digits of form fields, then I'd suggest this is the wrong tool for the job.

This issue is also based on the limitations of the ORM; yes, the entire module could be re-written, or the ORM could be removed and raw SQL queries used instead, but that will make the system more brittle in other ways and increase maintenance cost to the benefit only to those who want to use significant number of fields and to the cost of everyone else.

What's great about open source is, if you have a legitimate need for 100s of fields, here's the basis for you to start something. You can optimise it for your need and share it back or just use it yourself, but this module makes no promises to be everything to everyone nor to support 50+ fields.

Given this issue has been open for 3 years, I wrote some optimisations to mitigate the problem (within reason effort vs reward balance), and now no one who "needs" all these fields has offered a patch, I think this issue should be closed. There doesn't appear to be internal or external appetite to fix this problem and we've only had 2 complaints about it in 3 years - suggesting it doesn't affect many people.

@michalkleiner
Copy link
Contributor

I'm on the fence here. I'd say ~50 fields isn't that excessive, it's a 5 pages long form with 10 fields each page, not unreasonable. Triple digits might be seen as high.

We may also not know about these cases where it errors once but saves/publishes in the next try and CMS users may not see it/understand it as this very issue.

With some govt clients, we're seeing a slight increase in appetite to use the module trying to create all the online forms with it. We can surely debate if a form could/should be built bespoke or use UDF, at the end of the day it's the client making the call and our job is to explain the caveats to them. The flexibility and that they don't need to ask the vendor to build them a form every time is surely appealing to some.

It would probably help to steer clients towards bespoke forms if this module had a line in the README/the docs that it's not primarily intended or suitable for bigger forms and to always consult a dev on the feasibility of its use, or use it with that risk in mind.

@Firesphere
Copy link
Author

From @dhensby

Given this issue has been open for 3 years, I think this issue should be closed.

I strongly disagree, looking at how long some issues stay open. As for some PRs, due to the lack of resources from the maintainers.

From @michalkleiner

It would probably help to steer clients towards bespoke forms if this module had a line in the README/the docs that it's not primarily intended or suitable for bigger forms and to always consult a dev on the feasibility of its use, or use it with that risk in mind.

I again, would strongly disagree. Any organisation, give them the option to completely and freely manipulate forms, without having to turn back to the agency over and over again, will use that option.
When I opened this issue, it was only because I had the first client willing to pay for a solution to the issue.... Sadly, nothing came out of it. But it surely wasn't the first client that turned away from Silverstripe because of this issue.

@dhensby
Copy link
Contributor

dhensby commented May 14, 2021

I strongly disagree, looking at how long some issues stay open. As for some PRs, due to the lack of resources from the maintainers.

That's just a self-fulfilling argument; we can't oppose closing issues that have been open for 3 years because there are some that have been open longer. It's an arbitrary requirement that an issue can't be closed if there is an issue that's been open longer that doesn't form any of the official policy around issue management for this project.

When I opened this issue, it was only because I had the first client willing to pay for a solution to the issue

Well, that sounds like a good justification to close it then. The "only" reason you opened it is no longer applicable so it should be closed.

Ultimately, we can all have opinions about what the module "should" support in terms of number of pages/fields/conditions, but there's the reality that it only supports as many as your server and configuration can handle as it's currently architected. If a developer or their client has requirements beyond that, then that's for them to resolve, either through contribution to the project resolving the problem or through a bespoke solution.

We can add something to the README that is a statement of fact (because no matter what optimisations are made there will always be some upper-limit to how big a form can be before a server's resources / timeout is exhausted).

No one would be opposed to a backend refactor that was maintainable and optimised the logic allowing for hundreds or thousands of fields, but it's not forthcoming at the moment.

Frankly, if there are Govt. agencies using this for 100-field forms, they need to step-up and sponsor the improvements to the module to allow it.

@Firesphere
Copy link
Author

if there are Govt. agencies

I'll just leave this here.

How on Earth, is this a Govt agencies problem? It's a Silverstripe Userforms problem to me.

@dhensby
Copy link
Contributor

dhensby commented May 14, 2021

I don't understand your reasoning. Whoever's "problem" it is, is academic and irrelevant.

If you buy a car and try to drive it for 500 miles on one tank of petrol and it runs out at 450miles, who's problem is that, yours or the car's? Who cares, you need petrol. You can stand next to your car for 3 years shouting about how it should be able to go 500 miles on a tank of petrol (edit: hoping someone comes and fixes if for you for free). Or, you have two alternatives: Fix it yourself or pay someone to come fix it for you.

I'm going to lock this for the rest of the day and that can give some time to the maintainers to decide to close this or not. The bug and replication steps are here, it's not ambiguous to anyone, so there's no imminent need for further discussion on this point and it's digressing into a conversation about who's "problem" it is rather than being focused on the actual issue report

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

No branches or pull requests

6 participants