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

Review indices on mongodb #438

Open
Enkidu93 opened this issue Jul 18, 2024 · 6 comments
Open

Review indices on mongodb #438

Enkidu93 opened this issue Jul 18, 2024 · 6 comments
Assignees

Comments

@Enkidu93
Copy link
Collaborator

At some point, we should carefully comb through exiting indices specified in mongodb and see which are being used as well as give more thought to compound indices. We should also remove unused indices (both from the code and make sure to delete from Atlas).

@Enkidu93 Enkidu93 self-assigned this Jul 18, 2024
@ddaspit
Copy link
Contributor

ddaspit commented Jul 22, 2024

I would like to understand the rationale behind the recent index changes. Did we do any profiling to verify that these changes have a positive or negative impact?

@Enkidu93
Copy link
Collaborator Author

Yes, this is all the result of looking at the query profiler and index usage in Atlas. We were running into issues with queries on pretranslations examining way too many documents (because they weren't covered by existing indexes). Peeking now, I see that the same kind of query pre-update took almost 2 seconds and now it's taking around 200 ms. I removed redundant indexes, and I also removed indexes that were completely unused. (There was also a separate change regarding a bug in using a change stream that was causing a performance hit).

@ddaspit
Copy link
Contributor

ddaspit commented Jul 22, 2024

That sounds good. We probably want to re-add the index on the Webhook.Owner field. It probably isn't being used very much, but it is there to support the get all endpoint for webhooks.

@ddaspit
Copy link
Contributor

ddaspit commented Jul 22, 2024

Do we have an issue to fix the bug in using change streams?

@Enkidu93
Copy link
Collaborator Author

That sounds good. We probably want to re-add the index on the Webhook.Owner field. It probably isn't being used very much, but it is there to support the get all endpoint for webhooks.

OK. That makes sense. I did wonder about that - I figured the lack of use might be more to do with the kinds of requests we're getting more than simply unneeded indexes. I can put in a PR to re-add those quickly.

Do we have an issue to fix the bug in using change streams?

I fixed the particular misuse in a recent PR and left a comment about ways in which we might improve the implementation of change stream use in general by using resume tokens (if we intend to keep the timeout functionality).

@ddaspit
Copy link
Contributor

ddaspit commented Jul 22, 2024

I think we want to keep the timeout functionality. We should probably create a separate issue to track fixing the change streams.

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

No branches or pull requests

2 participants