[SERVER-30476] Use of exception thrown by ChunkManager::findIntersectingChunk() to determine single shard targeting Created: 02/Aug/17  Updated: 06/Dec/22

Status: Backlog
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: James Wahlin Assignee: Backlog - Query Optimization
Resolution: Unresolved Votes: 0
Labels: neweng
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Query Optimization
Participants:

 Description   

There are several places where we call ChunkManager::findIntersectingChunk() as a fast-path check to determine whether a single shard can be targeted for query. If it can't (one case being when shard key collation doesn't match the request) a uassert is triggered, which we then catch and ignore.

We should refactor these call sites to not use exceptions as part of the non-error path.

Here are a few examples. All callers of ChunkManager::findIntersectingChunk() and ChunkManager::findIntersectingChunkWithSimpleCollation() should be audited as part of this work.

https://github.com/mongodb/mongo/blob/20c85d4848b4e4b3c88e1788eaff362143fffd20/src/mongo/s/chunk_manager.cpp#L126-L134
https://github.com/mongodb/mongo/blob/20c85d4848b4e4b3c88e1788eaff362143fffd20/src/mongo/s/commands/cluster_aggregate.cpp#L238-L243



 Comments   
Comment by Andy Schwerin [ 06/Aug/17 ]

I don't think this should just be thought of as a performance problem. Swallowing exceptions without examining the code like that is risky at best. This implementation makes assumptions about all future implementations of findIntersectingChunk.

Comment by Kaloian Manassiev [ 03/Aug/17 ]

Yes, 2 is a true error case due to user error so leaving uassert there is acceptable. For 1, I am not clear on the details - but is that a common scenario? Isn't that already slower because we need to cast the collection of the request or do we just resort to broadcast in this case (which is also slow)?

Comment by James Wahlin [ 02/Aug/17 ]

The callers I audited have confirmed that they have the shard key before calling. The cases where findIntersectingChunk() will uassert are:

  1. The request and the shard key collation do not match (a valid use case)
  2. A chunk is not found in the chunk map that contains the shard key (which may be a true error case?)
Comment by Kaloian Manassiev [ 02/Aug/17 ]

Is it the case that these methods expect the caller to have the shard key in the passed-in BSON or the check is more complex than that? If it is just a matter of checking for the presence of the shard key, we could prefix the hot paths with that.

Generated at Thu Feb 08 04:23:57 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.