[SERVER-49724] Make ReadThroughCache support StringData keys Created: 20/Jul/20 Updated: 29/Oct/23 Resolved: 06/Nov/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Sharding |
| Affects Version/s: | None |
| Fix Version/s: | 4.9.0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Tommaso Tocci | Assignee: | Jaume Moragues (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Sprint: | Sharding 2020-11-02, Sharding 2020-11-16 |
| Participants: |
| Description |
|
At the moment it is not possible to use StringData type as keys of an instance of ReadThroughCache. In particular the problem is that the underlying InvalidateingLRUCache data structure doesn't also support StringData type as key. |
| Comments |
| Comment by Githook User [ 06/Nov/20 ] |
|
Author: {'name': 'Jaume Moragues', 'email': 'jaume.moragues@mongodb.com'}Message: |
| Comment by Tommaso Tocci [ 29/Sep/20 ] |
|
matthew.saltz proposed a solution based on templates that allows to specialize the underlying map container of the ReadThroughCache depending on the type of the object you need to store in it. |
| Comment by Kaloian Manassiev [ 22/Jul/20 ] |
|
The only issue I see with how it is currently implemented is a potential performance overhead due to the cost of converting from StringData to std::string in order to lookup the entry from the database cache. If this shows-up in performance benchmarks, we will have to do this ticket, but for now I am booting it out of the project. |
| Comment by Tommaso Tocci [ 21/Jul/20 ] |
|
Seems good to me, but since kaloian.manassiev raised this issue in a CR and he was worried about the performance impact about using std::string, I would like to ear his opinion about this. |
| Comment by Kevin Pulo [ 21/Jul/20 ] |
|
I think the problem is likely to be that StringData doesn't own the underlying string storage (which could be in a std::string or somewhere pointed to by a char*), making it unsuitable for storing in a container. Can you use std::string instead (ie. and explicit copy any StringData to a std::string when necessary)? |