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

Slow queries on larger result sets #229

Open
ceit-each opened this issue Jan 11, 2022 · 5 comments
Open

Slow queries on larger result sets #229

ceit-each opened this issue Jan 11, 2022 · 5 comments

Comments

@ceit-each
Copy link

Hi there,

I am new to this addon after working with emberfire for a good while (looking to move on due to lack of maintenance). Appreciate the work and the addon looks great!
However, one thing I've noticed is that when using query with a result set in the 1000s there is a fairly significant difference in the amount of time it takes for both addons to return the documents.

In my simple test, to retrieve 2000 documents using this.store.query, both addons took roughly the following amount of time to return:
ember-cloud-firestore-adapter: ~ 18 seconds
emberfire: ~ 4-5 seconds

I also would periodically come across the following error:

[2022-01-11T14:25:46.640Z]  @firebase/firestore: Firestore (9.6.2): FirebaseError: [code=resource-exhausted]: Too many outstanding requests.
[2022-01-11T14:25:46.641Z]  @firebase/firestore: Firestore (9.6.2): Using maximum backoff delay to prevent overloading the backend.

I have only briefly looked at the code of the addon, but is the individual fetching of docs in query() -> findQueryRecords() in adapters/cloud-firestore-modular.ts the cause of these issues?

@mikkopaderes
Copy link
Owner

Yes, there are individual fetches after a query but if you notice, the query uses onSnapshot and does the individual fetching inside of it. Then at the very end, it does a unsubscribe to it.

Because we kept a listener running, the individual fetches will attempt to fetch from the cache making it resolve the request significantly faster.

To be honest, it's been a while since I did this simple benchmarking and I'm not sure if it's still the case or if Firestore has changed that behavior already. Maybe it's time for me to check that out soon.

@mikkopaderes
Copy link
Owner

mikkopaderes commented Jan 19, 2022

I've checked this again, and it does seem to not act as I would've remembered then. I'm gonna try to revisit the implementation to improve the querying.

@ceit-each
Copy link
Author

Great, thanks for looking into it!

@mikkopaderes
Copy link
Owner

mikkopaderes commented Feb 23, 2022

Yes, there are individual fetches after a query but if you notice, the query uses onSnapshot and does the individual fetching inside of it. Then at the very end, it does a unsubscribe to it.

Because we kept a listener running, the individual fetches will attempt to fetch from the cache making it resolve the request significantly faster.

To be honest, it's been a while since I did this simple benchmarking and I'm not sure if it's still the case or if Firestore has changed that behavior already. Maybe it's time for me to check that out soon.

After revisiting this again, I'm going to backtrack my previous statement. It does seem like the scenario I mentioned above is still the same. If you already have a query onSnapshot, any document onSnapshot that's part of that query will be fetched from the cache. However, a getDoc will not. Here's an example:

const colRef = collection(db, 'users');
const queryRef = query(colRef, where('field', '==', 'foo'));

onSnapshot(queryRef, (querySnapshot) => {
  // querySnapshot contains /users/12345
});

// If after some time, you do a doc fetch via `onSnapshot` for a document 
// already fetched by `onSnapshot` above, it will fetch from cache and finish the request immediately.
const docRef = doc(db, 'users', '12345');

onSnapshot(docRef, (docSnapshot) => { });

// However, doing this doesn't fetch from cache so it will take a little bit longer to finish this request
getDoc(docRef);

I'm not sure how you did your benchmark in order to cause that output. It would help if you could share a repo. Nevertheless, I'm still keen on pursuing the refactor because looking at my code now, I feel it's so crafty with the supposed optimizations that it's hard to understand what's happening underneath at first glance.

Reference
image

@mikkopaderes
Copy link
Owner

#237 introduced the refactor

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

No branches or pull requests

2 participants