[SERVER-26919] Move alwaysAllowWrites() out of ConfigServerTestFixture::setUp() Created: 04/Nov/16 Updated: 06/Dec/22 |
|
| Status: | Backlog |
| Project: | Core Server |
| Component/s: | Replication |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Siyuan Zhou | Assignee: | Backlog - Replication Team |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | neweng | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Assigned Teams: |
Replication
|
| Sprint: | Repl 2017-01-23, Repl 2017-02-13, Repl 2018-09-24, Repl 2018-10-08, Repl 2018-10-22 |
| Participants: |
| Description |
|
Some sharding tests need to write into collections in non-primary states. We should have the setupShards() method or equivalent set the flag on the mock repl coordinator just for the window where it actually needs to bypass the op observer. |
| Comments |
| Comment by Jack Mulrow [ 15/Oct/18 ] |
|
siyuan.zhou, that sounds good to me. From what I can tell there isn't any reason the two places from my review that use the RAII type can't rely on primary state instead. It may be a little tricky to revive my patch since the code has changed since my CR was approved, but it shouldn't be too bad. |
| Comment by Siyuan Zhou [ 01/Oct/18 ] |
|
I still think this is a nice code cleanup, since having "alwaysAllowWrites" on ReplicationCoordinatorMock allows writes on secondaries and diverges from the real server behavior. The reason why it ever exists is that a few sharding tests fail on writes even if the replication coordinator is primary. Those sharding tests set up shards by writing into the "config.shards" collections in secondary mode. One such failure happened in LegacyAddShardLogOpHandler according to the comments of my code review. "alwaysAllowWrites" is a workaround for this issue. This ticket makes the workaround nicer and smaller. Jack's CR includes the change to make most of the tests to rely on primary state and adds the RAII type for just two exceptions. jack.mulrow, do you think it would better to fix the sharding testing behavior to just rely on primary state and get rid of "alwaysAllowWrites"? This could be done by new engineers of either replication team or sharding team. |
| Comment by Gregory McKeon (Inactive) [ 22/Aug/18 ] |
|
Sending to repl to triage. |
| Comment by Jack Mulrow [ 22/Aug/18 ] |
|
greg.mckeon No, this was never actually committed. If I remember correctly, this was the final ticket I worked on when rotating on replication and I noticed an intermittent failure in my final evergreen patch so I didn't push, but since I had just started full-time on sharding I never ended up getting back to this to figure out the problem. |
| Comment by Gregory McKeon (Inactive) [ 22/Aug/18 ] |
|
jack.mulrow this appears to have a closed, LGTM'd CR but no commit - did this ever go in? |
| Comment by Gregory McKeon (Inactive) [ 22/Aug/18 ] |