[SERVER-37375] ServiceContextMongoDTest fixture shouldn't write out storage engine related files to disk Created: 28/Sep/18 Updated: 29/Oct/23 Resolved: 07/Jan/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Replication, Storage |
| Affects Version/s: | None |
| Fix Version/s: | 4.3.3 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | William Schultz (Inactive) | Assignee: | Siyuan Zhou |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | tig-unittests | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Operating System: | ALL |
| Sprint: | Repl 2019-12-30, Repl 2020-01-13 |
| Participants: |
| Description |
|
Whenever a ServiceContextMongoDTest fixture is constructed, it initializes a new storage engine. This storage engine initialization process does at least two disk writes: it creates a PID file for the running process and also writes out the storage engine metadata file (storage.bson). For unit test suites with lots of tests, these file writes occur every time a test is run. This can add an undesirable amount of overhead to unit test execution, and these disk writes don't seem necessary since the unit tests mostly use an ephemeral storage engine. We should try to avoid doing these writes in our unit test fixtures. |
| Comments |
| Comment by Githook User [ 07/Jan/20 ] |
|
Author: {'name': 'Siyuan Zhou', 'email': 'siyuan.zhou@mongodb.com', 'username': 'visualzhou'}Message: |
| Comment by Siyuan Zhou [ 20/Dec/19 ] |
|
This seems a small patch but has a meaningful impact. I wrote up the patch and reduced the running time of db_repl_coordinator_test including 358 test cases from around 11 seconds to 4 seconds. Assigning to myself. |
| Comment by Sara Williamson [ 19/Dec/19 ] |
|
We're closing this as won't fix right now, as we don't want to make a change in behavior that only applies to unit tests (as that might introduce risk/reduce our test coverage). In the future we might want to revisit this as part of a larger effort to analyze unit test performance to see if there are other solutions. |
| Comment by William Schultz (Inactive) [ 15/Oct/18 ] |
|
That's fair, but in general I still think this is a worthwhile ticket. Unit tests, fundamentally, should be very fast to run. The fact that they require an external library to achieve that seems undesirable. |
| Comment by Craig Homa [ 01/Oct/18 ] |
|
Hey milkie, the repl team would like to know if this would be something quick/easy for your team to fix? |
| Comment by Andy Schwerin [ 28/Sep/18 ] |
|
To clarify, the storage engine isn't always the emphemeralForTest storage engine, but it always uses a new temp directory, so unless we're unit testing recovery steps that involve those files, not writing them seems quite safe. |
| Comment by William Schultz (Inactive) [ 28/Sep/18 ] |
|
Furthermore, after a cursory local run, all existing unit tests appear to succeed even when these disk writes are removed. |
| Comment by William Schultz (Inactive) [ 28/Sep/18 ] |
|
By some rough estimates from local testing, these disk writes can add up to ~20-30 milliseconds of execution time to each test. For a unit test suite with ~100 tests, this is 2-3 seconds of extra execution time, just for set up logic. |