[SERVER-73395] Query on temporary resharding collection has plan cache entry after resharding completes Created: 27/Jan/23  Updated: 06/Feb/24

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

Type: Task Priority: Major - P3
Reporter: Haley Connelly Assignee: Sergi Mateo Bellido
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
related to SERVER-65492 Invalidate plan cache entries when th... Closed
related to SERVER-65502 Complete TODO listed in SERVER-60824 Closed
Assigned Teams:
Catalog and Routing
Sprint: Sharding EMEA 2023-02-20, Sharding EMEA 2023-03-06, Sharding EMEA 2023-03-20, Sharding EMEA 2023-04-03, Sharding EMEA 2023-04-17, Sharding EMEA 2023-05-01, Sharding EMEA 2023-05-15, Sharding EMEA 2023-05-29, Sharding EMEA 2023-06-12, Sharding EMEA 2023-06-26, Sharding EMEA 2023-07-10, Sharding EMEA 2023-07-24, Sharding EMEA 2023-08-07, Sharding EMEA 2023-08-21, Sharding EMEA 2023-09-04, Sharding EMEA 2023-09-18, Sharding EMEA 2023-10-02, Sharding EMEA 2023-10-16, Sharding EMEA 2023-10-30, CAR Team 2023-11-13, CAR Team 2023-11-27, CAR Team 2023-12-11, CAR Team 2023-12-25, CAR Team 2024-01-08, CAR Team 2024-01-22, CAR Team 2024-02-05, CAR Team 2024-02-19
Participants:

 Description   

Context: Many commits ago, a check was added to skip running findOne() on empty collections until non-blocking sort was enabled for clustered collections. According to the TODO, the check can now be removed from the resharding::data_copy::findDocWithHighestInsertedId.

Although the check may actually be an optimization we wish to keep, we should investigate the side effects in case other things in resharding change and it becomes an issue in the future. 

Reproduction:

  • Change: Remove the empty collection check here. The only difference: findDocWithHighestInsertedId runs a findOne() before returning boost::none on the empty, temporary resharding collection
  • Outcome: invalidate_plan_cache_entries_when_collection_generation_changes.js expects to find 0 plan cache entries for the resharded collA, but finds one entry and fails.

    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] [
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 	{
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"version" : "2",
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"queryHash" : "620FE6DB",
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"planCacheKey" : "AB020F45",
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"isActive" : true,
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"works" : NumberLong(0),
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"timeOfCreation" : ISODate("2023-01-26T15:58:10.228Z"),
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"cachedPlan" : {
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 			"slots" : "$$RESULT=s9 $$RID=s10 env: { s1 = TimeZoneDatabase(Europe/Prague...US/Michigan) (timeZoneDB), s2 = Nothing (SEARCH_META) }",
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 			"stages" : "[3] limit 1 \n[2] nlj inner [] [s7, s3, s4, s6, s5] \n    left \n        [1] nlj inner [s4, s5] [] \n            left \n                [1] project [s4 = \"_id_\", s5 = {\"_id\" : 1}] \n                [1] limit 1 \n                [1] coscan \n            right \n                [1] project [s3 = s8] \n                [1] ixseek KS(F0FE04) KS(0A0104) s6 s7 s8 [] @\"eb54438c-3161-4dd2-bb9b-5eac443ff9a8\" @\"_id_\" false \n    right \n        [2] limit 1 \n        [2] seek s7 s9 s10 s3 s4 s6 s5 [] @\"eb54438c-3161-4dd2-bb9b-5eac443ff9a8\" true false \n"
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		},
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"indexFilterSet" : false,
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"isPinned" : true,
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"estimatedSizeBytes" : NumberLong(5497),
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"host" : "ip-10-122-12-243:20040",
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 		"shard" : "invalidate_plan_cache_entries_when_collection_generation_changes-rs0"
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] 	}
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] ]
    [js_test:invalidate_plan_cache_entries_when_collection_generation_changes] [jsTest] ----
    

 

Question: Is it expected for the findOne() on the temporary resharding collection to remain as a plan cache entry? 

Hypothesis: The findOne() run on the temporary resharding collection and when the collection is renamed, collIIsDropped is false by default, and _cleanupBeforeInstallingNewCollectionMetadata does not clean up the plan cache entry (SERVER-65492). The temporary sharding collection is renamed to collA, and keeps the same UUID it had when the findOne() was issued. Then, when the test checks for the absence of plan cache entries tied to collA, it finds an entry with collA's new UUID.



 Comments   
Comment by Sergi Mateo Bellido [ 01/Mar/23 ]

RenameCollection and dropCollection are explicitly calling to clearFilteringMetadataForDroppedCollection, we should do the same for reshardCollection.

Comment by Sergi Mateo Bellido [ 01/Mar/23 ]

I believe that the real issue here is that for the temporary namespace we are not calling to clearFilteringMetadataForDroppedCollection but just to clearFilteringMetadata. This new function was introduced in SERVER-69134, as a hacky way to remove the metadata manager of a namespace when we know that it is not going to be used anymore.

Comment by Haley Connelly [ 27/Jan/23 ]

If we decide to keep the check where we return early if a collection is empty, this is a non-issue in todays code. 

cc max.hirschhorn@mongodb.com  since this is resharding related

Generated at Thu Feb 08 06:24:33 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.