search

cycleplanet / cycle-planet

Official Cycle Planet repo, an open source and non-profit network of long distance cyclers and those who want to host them.
https://cycleplanet.org/
MIT License
10 stars 4 forks source link

query POI markers from Firebase by geohash as needed #87

Closed reinierl closed 2 years ago

reinierl commented 2 years ago

Should speed up retrieval of markers in multiple pages because we'll only request them for the part of the map we're actually looking at. POI data can now be requested from the marker store for a country using loadPoiWithinCountry or a bounding box using loadPoiWithinBounds.

Just as I review the pull request I'm noticing getting the POI markers for a user might be broken - it seems we don't query those from Firestore but from state.landMarkers in the markers store. That object is no longer guaranteed to include all POI markers by the current user with these changes. We'll have to add a method on the markers store that queries POI markers for a user from Firebase, won't we @bidsinga ?

Also this pull request does not yet optimize anything about the way host information is queried for display on a map.

reinierl commented 2 years ago

I think this is good to be merged now @bidsinga !

bidsinga commented 2 years ago

Ok let's do it then! The only small improvement we can still think of is to increase the loading speed for loadPoiByIds, which takes up to a minute when I load the markers I added myself. But let's merge and deploy it for now.

reinierl commented 2 years ago

The slowness of loadPoiByIds is the reflection of that corner you painted yourself into that I was talking about on Slack. There is no intelligent fast search criterion I can use in Firestore to get the POI we need for that page; we can only query the IDs from Firebase and then get the documents with those IDs from Firestore. And Firestore doesn't seem very good at retrieving documents given a set of IDs.

I do think we can speed it up roughly 10x by fetching them 10 by 10 instead of 1 by 1 using .where(..., 'in', ...) instead of .where(..., '==', ...). Unfortunately no more than 10x because Firestore has a limit of 10 values at once with .where(..., 'in', ...).

Can you make a new issue about this slowness? Otherwise I can do it tonight.

bidsinga commented 2 years ago

created a new issue #88