Extremely slow query that generates an out of memory error #1530
Comments
|
Hi @mihaicosareanu-bolt, I do think this came in as part of #1452, where we upgraded the parser a bunch. I'm also not super familiar with the parser code, and that pull request brought in quite a few changes, so it could be hard to figure out exactly what broke it or how to fix it. But we'll be looking as well, let us know if you find anything. |
|
Thanks for looking into it! I will spend more time on it next week :) |
|
Alright, we've tracked it down to a PR to add unit number support (pelias/parser#87). Pelias doesn't fully support unit numbers at this time, so that functionality was not really doing much, as far as I know. As a stopgap measure, this branch of pelias/parser which removes that functionality appears to be working well. |
|
Thanks a lot Julius! Indeed, tested on my machine and it works without that code. Do you plan on retiring this bit until we have proper support of unit numbers? What are your thoughts? |
|
I'm not 100% sure what we'll do yet, though we'll definitely put in a fix one way or another. I'd like to discuss with @missinglink first, but he's in the middle of some fairly disruptive travel, so it will be a few days before we can chat about it. |
|
Great! Looking forward to your conclusion :). Safe travels @missinglink :D |
This is a temporary fix for issues found parsing some longer inputs. We don't think the Unit number parsing is especially to blame, it's just the "last thing" that was added and increases the number of parser solution combinations. Since unit number support in Pelias is still in progress, it's a reasonble thing to remove as a temporary workaround until we have a more permanent fix. Connects pelias/api#1530
A performance issue in the Pelias Parser where certain inputs can cause long parse times and out of memory errors was [recently reported](#1530). This change implements a short term fix by disabling some functionality around unit number parsing that was contributing to the issue. From our analysis it's not so much that the unit number parsing _caused_ the issue, but it was the most recently added functionality that contributed to it. While we develop a longer term fix, which should allow us to keep unit number parsing around, this "hotfix" will keep everything running smoothly. Unit numbers are not yet fully supported by Pelias so there _shouldn't_ be any major loss in functionality. Fixes #1530
|
Okay, we've just put in a 'hotfix' for this issue, and it should be effectively resolved with no regressions in result quality. After talking with @missinglink there are a bunch of different ways we can put in a long term fix, which we can discuss here. For now though, anyone using the |
This is a temporary fix for issues found parsing some longer inputs. We don't think the Unit number parsing is especially to blame, it's just the "last thing" that was added and increases the number of parser solution combinations. Since unit number support in Pelias is still in progress, it's a reasonble thing to remove as a temporary workaround until we have a more permanent fix. Connects pelias/api#1530
This is a temporary fix for issues found parsing some longer inputs. We don't think the Unit number parsing is especially to blame, it's just the "last thing" that was added and increases the number of parser solution combinations. Since unit number support in Pelias is still in progress, it's a reasonble thing to remove as a temporary workaround until we have a more permanent fix. Connects pelias/api#1530
This PR removes the unit number parsing functionality which was causing OOM errors in some situations. see: pelias/api#1530
This PR removes the unit number parsing functionality which was causing OOM errors in some situations. see: pelias/api#1530
BREAKING CHANGE: remove unit classifier This PR removes the unit number parsing functionality which was causing OOM errors in some situations. see: pelias/api#1530
BREAKING CHANGE: remove unit classifier This PR removes the unit number parsing functionality which was causing OOM errors in some situations. see: pelias/api#1530
BREAKING CHANGE: remove unit classifier This PR removes the unit number parsing functionality which was causing OOM errors in some situations. see: pelias/api#1530
Describe the bug
I have found a query that brings pelias-api to its knees. I'm currently trying to understand the code behind the Tokenizer and ExclusiveCartesianSolver, but I thought I should give you a heads up as well, maybe you will understand much faster what happens. I've tried other queries with multiple tokens but none behave as bad as this one :).
Steps to Reproduce
(link redacted for somewhat obvious reasons. Feel free to look at the edit history if you are interested but take care to not hurt your own or others' Pelias servers. Not everyone has patched yet 😁)
What I've noticed is that in a previous pelias version (tried a commit from 29 Jun 2020) it works fast. So there were some changes in between that have affected this (on my machine the latest master returns in 22 seconds vs 800 ms in the 2020 June commit).
Expected behavior
Should return as fast as other queries with multiple tokens do.
Environment (please complete the following information):
Linux & Mac
Pastebin/Screenshots
Additional context
References
The text was updated successfully, but these errors were encountered: