search

silverstripe / silverstripe-userforms

UserForms module provides a visual form builder for the Silverstripe CMS. No coding required to build forms such as contact pages.
BSD 3-Clause "New" or "Revised" License
135 stars 226 forks source link

Saving UserForms with many fields causes timeouts #773

Open Firesphere opened 6 years ago

Firesphere commented 6 years ago

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 commented 6 years ago

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

dhensby commented 6 years ago

see https://github.com/silverstripe/silverstripe-userforms/pull/538

Firesphere commented 6 years ago

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

dhensby commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

Firesphere commented 6 years ago

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 commented 4 years ago

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

Firesphere commented 4 years ago

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?

bergice commented 4 years ago

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

Firesphere commented 4 years ago

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 commented 4 years ago

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:

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

Optimised Execution Times

(FileLinkTracking, SiteTreeLinkTracking, AssetControlExtension extensions disabled)

Firesphere commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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