search

inspiredminds / contao-sibling-navigation

Provides a previous/next navigation for news & events.
GNU Lesser General Public License v3.0
6 stars 0 forks source link

memory overflow with recurring events #3

Closed joehi closed 5 years ago

joehi commented 5 years ago

Contao 4.7.4 contao-sibling-navigation 1.1.5

When displayed event is not in list of events selected for module sibling navigation it generates an error 500. There is no entry in the log file.

It should be allowed as it might happen by accident or by intention.

My idea is to move cancelled events to a separate events list so they do not get displayed in the normal list anymore but that the page of the event itself still exists and the URL stays valid to show to visitors instead of an error 404 the (updated) information that the event got cancelled.

One (preferred) solution would be to generate links to the events who are before and after the specific date of this displayed event.

Alternatively it could be possible in this case to generate no links at all so that the module sibling navigation just generates no output.

So I guess it is a mixture of a bug and a feature request.

fritzmg commented 5 years ago

Hm, I think I should go with this:

Alternatively it could be possible in this case to generate no links at all so that the module sibling navigation just generates no output.

fritzmg commented 5 years ago

I cannot actually reproduce the problem. The 500 error must be caused by something else.

fritzmg commented 5 years ago

Btw. the current behavior is, that the sibling navigation automatically provides a navigation within the event's calendar, if the event is not part of the selected calendars:

https://github.com/inspiredminds/contao-sibling-navigation/blob/e341c180d57bc363fd7fdddb985479453e67200b/src/Module/SiblingNavigationEvents.php#L85-L89

joehi commented 5 years ago

Hm, I tried it with another installation on another server and there I cannot reproduce it either. That is weird. I will try to further investigate. As soon as I disable the module sibling navigation on the page the error is gone. That's why I suspected it to have a bug.

I am using changelanguage for a bilingual website on both installations.

And in that second installation I noticed that I just set up the German calendar in the module configuration but it works both for navigation within the German calendar as well as within the English calendar without having to set up another module configured for the English calendar.

fritzmg commented 5 years ago

Try the current master branch please. I made some changes in f349dd24cb5d4b9bef5f6978341c3d3053e14bb3 which could potentially solve some problems. Though I doubt that it would fix anything in your case.

joehi commented 5 years ago

On a third installation I managed to reproduce it again. The server runs with php 7.3.4 and MariaDB 10.3.14 where these errors occur.

The other server with php 7.2.17 and mariaDB 10.1.37 does not produce this error.

Which version of php and especially mariaDB did you use for testing?

I wonder if this effect has to do with the new version of mariaDB as is in issue #436 in contao/contao.

joehi commented 5 years ago

I installed the current master branch with f349dd2 but the error is still there.

fritzmg commented 5 years ago

Are you able to produce a stack trace via the app_dev.php?

joehi commented 5 years ago

I think I found the reason. It was just hiding and leading me a totally wrong direction.

The problem occurs as soon as there is a recurring event in the calendar.

And it happens on different Contao installations on different servers. So it has nothing to do with the version of php or mariaDB.

Shall we update the title and reopen the issue? Or is it better and less confusing to just open a new issue this time with the right title and description?

fritzmg commented 5 years ago

I still need an exact error or reproduction at least ;)

fritzmg commented 5 years ago

Ah, I think I know what could be the problem. If you have events, that repeat to infinity, then they are also generated to "infinity" - which may cause a script timeout or a memory overflow.

Not sure how to handle it though.

fritzmg commented 5 years ago

@joehi can you confirm, that the 500 error is either script timeout or memory overflow? You need to check your logs.

joehi commented 5 years ago

How to reproduce

Create a calendar with some events, non of thse events recurring. Create a page to display the individual event with an event reader module and a sibling navigation module. Everything works as expected.

Now add an event (or change an event) with Recurrence settings activated to repeat the event. Recurrences is set to 0 (default) for unlimited recurrences. When you open the page to display an individual event, it will create an error 500 now.

It is enough to try to diplay any event of the calendar which includes one recurring event. It will always create an error 500.

On a server with 2 GB of RAM I get this entry in the Contao log:

[2019-04-19 08:28:27] request.CRITICAL: Uncaught PHP Exception Symfony\Component\Debug\Exception\OutOfMemoryException: "Error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 20480 bytes)" at /directory/contao/vendor/contao/core-bundle/src/Resources/contao/library/Contao/StringUtil.php line 1060 {"exception":"[object] (Symfony\\Component\\Debug\\Exception\\OutOfMemoryException(code: 0): Error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 20480 bytes) at /directory/contao/vendor/contao/core-bundle/src/Resources/contao/library/Contao/StringUtil.php:1060)"} []

On a server with 4 GB of RAM I do not get any entry into the Contao log.

fritzmg commented 5 years ago

I wasn't able to reproduce it with just one (daily) recurring event. I needed several events (depending on the start date) until the memory consumption reached 128 MiB.

You need to increase your php memory_limit. Your current memory_limit is only 128 MiB - which is not a lot by today's standards. The more memory PHP has available, the more recurring events you can handle.

Otherwise I can only offer to implement an option to limit the events to either past or future events for the sibling navigation. But even then you could reach your memory limit, depending on the number of recurrences.

joehi commented 5 years ago

I increased the php memory_limit to 1024MB. Now I do not get any more entries in the contao logfile but loading the page still generates an error 500. I have just one recurring event in the calendar.

fritzmg commented 5 years ago

You need to find a log entry for that 500 error.

joehi commented 5 years ago

The only entry I found so far after increasing the php memory_limit tp 1024 MB is the apache log which just tells that the status code of the delivered page was 500. There is no apache error log entry.

If I change the number of recurrences from infinit to a fixed number the page loads. And I noticed that sibling navigation anyway just links the first entry of the recurring events as their URL is the same whichever occurence it is. With other words: Right now there is no difference regarding the FE output of the module sibling navigation whether it is an recurring event or not.

So is it necessary for the software to browse through this loop of recurrences at all?

fritzmg commented 5 years ago

With other words: Right now there is no difference regarding the FE output of the module sibling navigation whether it is an recurring event or not.

Yes, that is intentional. However, the order could be improved upon.

So is it necessary for the software to browse through this loop of recurrences at all?

Yes, otherwise recurrences will not be taken into account.

The only entry I found so far after increasing the php memory_limit tp 1024 MB is the apache log which just tells that the status code of the delivered page was 500. There is no apache error log entry.

There must be an error long some where. If it is not the memory_limit, may be it is the max_execution_time. How long does the page take to load? What is your current max_execution_time? You can check your PHP configuration via the developer environment (app_dev.php).

joehi commented 5 years ago

Unfortunately also using the developer environment delivers just an error 500 without any further information as at that moment there is not development bar shown on the page.

Loading the page takes a few seconds (less than 10 seconds) until the error 500 gets displayed. The php max_execution_time was set to 30 seconds. Increasing it to 120 seconds did not help.

joehi commented 5 years ago

With other words: Right now there is no difference regarding the FE output of the module sibling navigation whether it is an recurring event or not.

Yes, that is intentional. However, the order could be improved upon.

So is it necessary for the software to browse through this loop of recurrences at all?

Yes, otherwise recurrences will not be taken into account.

I understand, as soon as recurrences will be taken into account it is necessary to browse through this loop.

But in the special case (as it is right now) that only the first occurence of the event will be used to determine the order, it would not be necessary to browse through this loop. The order is the same as if there would be no recurrences at all.

fritzmg commented 5 years ago

Make the following test:

Just to check if the same error occurs with the regular Contao event list.

joehi commented 5 years ago

The event list module (showing all events) is not the problem. It will list all the events up to the year 2038. The problem occurs as soon as you click on any event in this list which will then be opened in the event reader.

As long as on this reader page there is only the module EVENT READER included, everything is fine and I can open the page in the FE without errors.

Now I will add the module EVENT SIBLING NAVIGATION to the page. Then I get the error when opening the page in the FE.

As soon as I disable the module EVENT SIBLING NAVIGATION for the page in the BE, it is possible to open the page in the FE again without errors.

fritzmg commented 5 years ago

The event list module (showing all events) is not the problem. It will list all the events up to the year 2038.

Ok, try the current master branch. I had PHP_INT_MAX as the upper limit for the events, unlike Contao's limit to 2038-01-01 00:00:00.

joehi commented 5 years ago

Ok, try the current master branch. I had PHP_INT_MAX as the upper limit for the events, unlike Contao's limit to 2038-01-01 00:00:00.

Perfect, that solved the problem. Now I cannot reproduce the error in that updated installation any more.

fritzmg commented 5 years ago

Ok :). Then I'll release this as a bug fix.

joehi commented 5 years ago

Thank you for your time and support finding and solving this limit case.