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 operations reported by Atlas #374

Closed
bourquep opened this issue Nov 20, 2023 · 21 comments
Closed

Slow operations reported by Atlas #374

bourquep opened this issue Nov 20, 2023 · 21 comments

Comments

@bourquep
Copy link
Contributor

Using Hangfire.Mongo v1.9.14 in production, on a Mongo Atlas M40 cluster.

The Atlas Profiler reports a lot of slow operations for the jobGraph collection (5-6 seconds to complete):

CleanShot 2023-11-20 at 15 15 50@2x

Most of those slow operations are reported for this query:

    "find": "Contents.jobGraph",
    "filter": {
      "_t": "ListDto"
    },
    "projection": {
      "_id": 1
    },

Which in my case returns 227k documents.

An explain on this query shows:

CleanShot 2023-11-20 at 15 26 21@2x

I believe that this long query is invoked by this code:

https://github.com/Hangfire-Mongo/Hangfire.Mongo/blob/b411aa164573c455e3b8670abb8019ee6c462d2e/src/Hangfire.Mongo/MongoWriteOnlyTransaction.cs#L338-L344

The enclosing method, TrimList(), has a key argument which is not used as a predicate in the Mongo query, but rather as a Where clause in a code manipulation of the returned list.

I wonder if refactoring the code to include the key in the Mongo query would help?

@bourquep
Copy link
Contributor Author

I have created the following index, and it seems to help (anecdotal evidence by explaining a few manual queries, will have to let it run for a few hours to see what Atlas Profiler has to report):

{
  _t: 1,
  _id: 1
}

Adding _id to the index makes the query/projection covered by the index, meaning there is no need for a FETCH stage:

CleanShot 2023-11-21 at 15 06 25@2x

I'll get back to you in a few hours with Atlas Profiler results!

@bourquep
Copy link
Contributor Author

So far so good!

CleanShot 2023-11-21 at 19 35 37@2x

@bourquep
Copy link
Contributor Author

The index definitely helps! (last 12 hours):

CleanShot 2023-11-22 at 09 52 48@2x

@gottscj
Copy link
Owner

gottscj commented Nov 27, 2023

@bourquep,

Thank you for the analysis!
While I Definity will be adding the index, I'm curious as to why there are so many ListDto items (227K) in the first place. My expectation was that they would be removed as they should expire.

Would it be possible for you to share your ListDto's with me? The 227K ListDto's only should be fine.
Also I will be looking into doing this in an aggregation pipeline instead. It has always been a bit of a quick and dirty solution. :)

Thanks!

@bourquep
Copy link
Contributor Author

I'm curious as to why there are so many ListDto items (227K) in the first place. My expectation was that they would be removed as they should expire.

We use batches a lot, maybe it is related? They do expire and they are removed.

Would it be possible for you to share your ListDto's with me?

Sure, here they are!

studyo-hangfire.Contents.jobGraph.json.zip

@gottscj
Copy link
Owner

gottscj commented Nov 27, 2023

Great thanks!

@gottscj
Copy link
Owner

gottscj commented Nov 27, 2023

@bourquep,

Looking at the ids query.

https://github.com/Hangfire-Mongo/Hangfire.Mongo/blob/b411aa164573c455e3b8670abb8019ee6c462d2e/src/Hangfire.Mongo/MongoWriteOnlyTransaction.cs#L338-L343

there seems to be room for further improvement in the filter as I did not take the key into account. so instead of fetching ALL ids, I can limit the count to the context of the given key like so:

 // get all ids for given key
 var filter = new BsonDocument
{
    ["_t"] = nameof(ListDto),
    [nameof(ListDto.Item)] = key
};
 var allIds = DbContext.JobGraph
      .Find(filter)
      .Project(new BsonDocument("_id", 1))
      .ToList()
      .Select(b => b["_id"].AsObjectId)
      .ToList();

looking at your collection the max returned documents would be 5 for a given key.

@bourquep
Copy link
Contributor Author

@gottscj Nice find! Then make sure to have an index on _t, key, _id

gottscj added a commit that referenced this issue Nov 28, 2023
fix Dashboard does not show exception for retries (#375)
fix Slow operations reported by Atlas (#374)
gottscj added a commit that referenced this issue Nov 28, 2023
* optimize trimming list and show retry info in dashboard

fix Dashboard does not show exception for retries (#375)
fix Slow operations reported by Atlas (#374)

* update changelog and version
@gottscj
Copy link
Owner

gottscj commented Nov 28, 2023

@bourquep,

I released a patch. please let me know if it works for you.

Thanks!

@bourquep
Copy link
Contributor Author

Looks like you forgot to create the index 😉

@gottscj
Copy link
Owner

gottscj commented Nov 29, 2023

@bourquep,

I do have sparse indexes for "_t" and "Item", which I hoped was enough. Is Atlas recommending further indexes?

Thanks!

@bourquep
Copy link
Contributor Author

Yes, since you're projecting _id, including the field in the index will make the query "covered", meaning Mongo won't have to fetch the documents in order to project the _id.

@gottscj
Copy link
Owner

gottscj commented Nov 30, 2023

Great!

Thank you for the help!

@gottscj gottscj closed this as completed Nov 30, 2023
@bourquep
Copy link
Contributor Author

bourquep commented Dec 4, 2023

@gottscj So you've decided not to create the index?

@gottscj
Copy link
Owner

gottscj commented Dec 4, 2023

@bourquep,

I was under the impression the current indexes was sufficient? Did I misunderstand?
If there optimizations to be made, I'm all in!

For some reason ListDto has a field "Item" instead of "Key".
Renaming "Item" to "Key" would make more sense and it would be able to use the
_t, Key compound index

Your suggested index _t, key, _id would be problematic as ListDto does not have a "Key" field.

What are your index recommendations now that the filter has been updated?

var filter = new BsonDocument
 {
      ["_t"] = nameof(ListDto),
      [nameof(ListDto.Item)] = key
 };

Thanks!

@gottscj gottscj reopened this Dec 4, 2023
@bourquep
Copy link
Contributor Author

bourquep commented Dec 4, 2023

My only recommendation was to make sure _id was part of the index, so the projection can be "covered".

@gottscj
Copy link
Owner

gottscj commented Dec 6, 2023

reviewing your given collection of ListDto's (165940 items), I think it should work satisfactory as we are now filtering before projecting returning only the ListDto's referenced by the "key" parameter.

Have you experienced any slow queries in your system?

Thanks!

@bourquep
Copy link
Contributor Author

bourquep commented Dec 6, 2023

I haven't updated to 1.9.15 yet, was waiting for the index but I'll deploy the update in the coming days and let you know.

@gottscj
Copy link
Owner

gottscj commented Dec 6, 2023

thanks! 🙏

@bourquep
Copy link
Contributor Author

I have updated to 1.9.15 at 11:00 this morning, and it makes a huge difference!

CleanShot 2023-12-12 at 12 54 46@2x

@gottscj
Copy link
Owner

gottscj commented Jan 4, 2024

@bourquep,

Thank you for the feedback!
I will close this issue then. :)

@gottscj gottscj closed this as completed Jan 4, 2024
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