[SERVER-44540] Rename forceRoutingTableRefresh to reflect the refresh of filtering metadata Created: 11/Nov/19 Updated: 29/Oct/23 Resolved: 10/Dec/19 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Sharding |
| Affects Version/s: | None |
| Fix Version/s: | 4.3.3 |
| Type: | Improvement | Priority: | Minor - P4 |
| Reporter: | Josef Ahmad | Assignee: | Tommaso Tocci |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Sprint: | Sharding 2019-12-16 |
| Participants: |
| Description |
|
The command name forceRoutingTableRefresh is misleading because the command actually refreshes the filtering metadata, not the routing metadata. |
| Comments |
| Comment by Githook User [ 10/Dec/19 ] |
|
Author: {'name': 'Tommaso Tocci', 'username': 'toto-dev', 'email': 'tommaso.tocci@10gen.com'}Message: Replace deprecated `forceRoutingTableRefresh` call with `_flushRoutingTableCacheUpdate`. There are no more calls to the deprecated `forceRoutingTableRefresh`, |
| Comment by Kaloian Manassiev [ 19/Nov/19 ] |
Ah of course, I forgot about that command that we introduced as well. Is it a problem that we refresh all shards which receive chunks from a collection during initial sharding? I'd say it definitely doesn't hurt, but also you are right that we shouldn't need it for correctness. |
| Comment by Esha Maharishi (Inactive) [ 18/Nov/19 ] |
|
kaloian.manassiev, Hmm, I don't think the _flushRoutingTableCacheUpdates call would create indexes and clone options... I believe that is done through this call to cloneCollectionOptionsFromPrimaryShard. Otherwise, the plan to remove uses of the {forceRoutingTableRefresh} name in 4.4 so that we can add a new name in 4.6 sounds good to me. |
| Comment by Kaloian Manassiev [ 18/Nov/19 ] |
It's not just the refreshing of the metadata, but we also need to create the indexes and clone collMods, which happens under that call as well. |
| Comment by Kaloian Manassiev [ 18/Nov/19 ] |
|
Ah, I didn't realise that there are already two different users of this name. This means that it would take one release to unify them and drop forceRoutingTableRefresh and then one more release to do the complete renames. Given that the name of the command is just a minor nuisance, I'd say let's leave it as it is now, but at least in 4.4 make everyone use the _flushRoutingTableCacheUpdates variant, so we can drop forceRoutingTableRefresh in 4.6. We can document in the TSE KBs that they should use the _flushRoutingTableCacheUpdates variant. josef.ahmad, do you have any objections to this plan? |
| Comment by Esha Maharishi (Inactive) [ 18/Nov/19 ] |
|
kaloian.manassiev ok, sure. Note that we renamed it once to "_flushRoutingTableCacheUpdates," and right now both names are in use:
It might be a bit tricky to give it a third name, given that these commands are sent between nodes (so we would have to take care that mixed-version clusters still work), and commands only support one alternate name today. (By the way, is it required that _shardsvrShardCollection make the other shards refresh when doing initial split? It seems like only the primary shard should need to refresh...) |
| Comment by Kaloian Manassiev [ 15/Nov/19 ] |
|
esha.maharishi, the issue is just the name of the command, not what happens behind the scenes. The name of the command indicates that the routing metadata is being refreshed, while in fact the intention of the command is to refresh the filtering metadata I propose we rename it to _shardsvrSyncFilteringMetadata - what do you say? |
| Comment by Esha Maharishi (Inactive) [ 14/Nov/19 ] |
|
josef.ahmad, right now the filtering metadata is actually refreshed by refreshing the routing metadata and then transferring the new chunk distribution into the filtering metadata. |
| Comment by Carl Champain (Inactive) [ 11/Nov/19 ] |
|
Hi josef.ahmad, Passing this ticket along to the Sharding team. |