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

Memory leak in floating bike updater #3316

Closed
abyrd opened this issue Jan 14, 2021 · 2 comments · Fixed by #3351
Closed

Memory leak in floating bike updater #3316

abyrd opened this issue Jan 14, 2021 · 2 comments · Fixed by #3351

Comments

@abyrd
Copy link
Member

abyrd commented Jan 14, 2021

The bike updater splits street edges to insert bike stations, but doesn't reverse that process when stations are removed. There is a comment about this from 2013 on BikeRentalUpdater.BikeRentalGraphWriterRunnable#run saying TODO: need to unsplit any streets that were split. The lack of edge removal was probably not a major problem when infrequently loading lists of fixed multi-bike stations, as those change very rarely. But free-floating bikes were introduced around 2017, which involve very frequent additions and removals of bike "stations" for individual bicycles. @gmellemstrand has pointed out that free floating bike support is enabled by default causing a memory leak. He is planning to disable it by default until split edge removal is implemented. Ideally that removal should use a generalized solution that also handles other temporary edges.

@gmellemstrand is also planning to link from migration guide and/or configuration guide with a short explanation of why this is disabled.

@sven4all
Copy link
Contributor

sven4all commented Jan 14, 2021

Possibly this issue is related #3273, we use the GBFS importer at this moment.

@abyrd
Copy link
Member Author

abyrd commented Jan 14, 2021

This seems to be the same issue @evansiroky fixed in #2791, where he described the problem in great detail. So this is the same problem described in #2787, which was somehow closed by a merge to a fork that referenced an otp ticket.

The pull request #2791 is built on top of another pull request #2762. In late 2019 there was a good deal of discussion about the implications of these fairly large changesets. The changes to the street splitting process seem to fix important flaws in design or usage of the splitter. But it looks like more work and discussion may be necessary to understand the nature of the changes and ensure they're correct and maintainable.

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

Successfully merging a pull request may close this issue.

2 participants