[SERVER-72363] Consider removing CollectionStatistics::addHistogram method Created: 22/Dec/22 Updated: 19/Jan/23 Resolved: 19/Jan/23 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Anton Korshunov | Assignee: | Misha Tyulenev |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Assigned Teams: |
Query Optimization
|
| Sprint: | QO 2023-01-23 |
| Participants: |
| Description |
|
A CollectionStatistics is not concerned about the source of histograms. E.g., CollectionStatisticsImpl should get its data from from the stats cache that is accessed when CollectionStatisticsImpl::getHistogram is called. While CollectionStatisticsMock used it tests should provide a more flexible interface to return custom histograms. That said, it seem that CollectionStatistics::addHistogram should not be part of the CollectionStatistics interface and we may consider removing it. To facilitate testing the addHistogram method can be kept in the CollectionStatisticsMock class. Another idea proposed by Misha would be changing addHistogram to a private _addHistogram and adding addHistogramForTest to clarify that this API is only for testing. In addition, currently when addHistogram is called providing a path for which a histogram already exists, we silently replace the old entry. If this behaviour is fine, we may consider changing the name to addOrReplaceHistogram, or otherwise raise an error. |
| Comments |
| Comment by Misha Tyulenev [ 19/Jan/23 ] |
|
There are unit tests that depend on the CollectionStatistics interface - namely histogram_estimator_test and benchmark_test. Removing the addHistogram will force the tests use CollectionStatisticsMock instead |