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

Fix Boundingbox Query Index Type #186

Conversation

remuslazar
Copy link
Contributor

This fixes a regression caused by bc61dca.

See #170 and related for details.

In a nutshell

bc61dca changes the index type to be a "2dsphere" but this will not be used at all for the bounding box queries currently generated by the app logic.

This fixes a regression introduced by bc61dca.

See openchargemap#170 and related
@remuslazar
Copy link
Contributor Author

@webprofusion-chrisc I was about to streamline https://github.com/ev-freaks/ocm-mirror to use the latest codebase and .NET 6 and so I ran into this issue.

@remuslazar remuslazar changed the title Use a 2d index for "SpatialPosition.coordinates" Fix Boundingbox Query Index Type Nov 10, 2021
@webprofusion-chrisc
Copy link
Member

Thanks, I think the reason I avoided this change was the API results (searching around a coordinate ) didn't work as expected. I'd need to re-test this.

@remuslazar
Copy link
Contributor Author

Thanks, I think the reason I avoided this change was the API results (searching around a coordinate ) didn't work as expected. I'd need to re-test this.

for the nearby (radius based) queries there is another index in place, so this change should have no side effect there.

@webprofusion-chrisc
Copy link
Member

I've tested this again and the index is not being used, I need to upgrade the C# driver to the modern version (because our search within a polygon gets translated to $within > $polygon instead of $geoWithin > $geometry, which is a bunch of changes. If it can be made to actually use the index it goes from scanning all documents (300ms) to using the index (1ms), there is other stuff to do with any additional filters (connection type etc), which suggests we should probably perform the spatial query then filter the remaining items in memory.

@remuslazar
Copy link
Contributor Author

I've tested this again and the index is not being used, I need to upgrade the C# driver to the modern version (because our search within a polygon gets translated to $within > $polygon instead of $geoWithin > $geometry
(..)

Correct, see #170 (comment).

Should this here then be merged, the refactoring to use the modern driver etc. could be done separately then. What do you think?

@remuslazar
Copy link
Contributor Author

.. alternatively I could change the PR to just add an additional index, keeping the existing 2dsphere index around. But this will not make a lot of sense IMO, the 2dsphere index not being used at all. This will be confusing..

@webprofusion-chrisc
Copy link
Member

It's OK, I'm working on the rewrite to the newer MongoDB driver now, then I'll ensure we are providing indexes for the new queries. These changes will fix a few other things as we've struggled with the old code now being async for a while (causing blocked threads and in turn thread pool exhaustion).

@webprofusion-chrisc
Copy link
Member

webprofusion-chrisc commented Nov 16, 2021

Closing. The MongoDB caching has been substantially updated now. Local API queries are down to 42ms from 250ms, mainly due to using the index. There is still plenty of room for improvement in performance and accuracy. [Edit: this is using MongoDB 5.0.3 in production and dev]

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 this pull request may close these issues.

2 participants