[SERVER-47142] Check primary before writing replset config and no-op Created: 26/Mar/20 Updated: 29/Oct/23 Resolved: 17/Apr/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Replication |
| Affects Version/s: | None |
| Fix Version/s: | 4.4.0-rc3, 4.7.0 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Pavithra Vetriselvan | Assignee: | Siyuan Zhou |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Operating System: | ALL | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Backport Requested: |
v4.4
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| Sprint: | Repl 2020-04-06, Repl 2020-04-20 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Participants: | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| Linked BF Score: | 42 | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
There are currently two problems. 1) We do not check if we are still primary before writing down a new config document locally. Consider the following scenario:
Node1's config will continue to get propagated via heartbeats even though it already stepped down. 2) The replSetReconfig command does a no-op write, but does not check that the node is still primary before doing so (Similar example, readConcern: linearizable) We end up calling onInternalOpMessage, which will pass in an empty namespace. Because of this, we don't actually do the primary check in _logOpsInner. This would mean that we can allow the reconfig no-op write to occur on a secondary. Since these two things should happen together to avoid any inconsistent states, we should consider refactoring the code so we can do the primary check once. |
| Comments |
| Comment by Githook User [ 14/May/20 ] |
|
Author: {'name': 'Judah Schvimer', 'email': 'judah@mongodb.com', 'username': 'judahschvimer'}Message: |
| Comment by Githook User [ 24/Apr/20 ] |
|
Author: {'name': 'Siyuan Zhou', 'email': 'siyuan.zhou@mongodb.com', 'username': 'visualzhou'}Message: (cherry picked from commit 31a60aab7a3d6b93407e0447b056c33f31a15991) |
| Comment by Githook User [ 24/Apr/20 ] |
|
Author: {'name': 'Siyuan Zhou', 'email': 'siyuan.zhou@mongodb.com', 'username': 'visualzhou'}Message: (cherry picked from commit 544cbb209709ebee4f17f2d669b1909bf66be6bb) |
| Comment by Githook User [ 17/Apr/20 ] |
|
Author: {'name': 'Siyuan Zhou', 'email': 'siyuan.zhou@mongodb.com', 'username': 'visualzhou'}Message: |
| Comment by Githook User [ 17/Apr/20 ] |
|
Author: {'name': 'Siyuan Zhou', 'email': 'siyuan.zhou@mongodb.com', 'username': 'visualzhou'}Message: |
| Comment by Githook User [ 17/Apr/20 ] |
|
Author: {'name': 'Siyuan Zhou', 'email': 'siyuan.zhou@mongodb.com', 'username': 'visualzhou'}Message: This reverts commit 0e7b476357a12802f1be1ca5c8a4b1a919000ef8. |
| Comment by Siyuan Zhou [ 09/Apr/20 ] |
|
william.schultz pointed out:
I'll address it in this ticket as well. |
| Comment by Siyuan Zhou [ 28/Mar/20 ] |
tess.avitabile, as we discussed today, we may not need to do anything in
Good point. I agree the fixes for both config write and no-op write are beginning-of-time bugs, but the race between reconfig and stepdown is extremely rare in earlier versions, so I'd prefer to keep them as-is and only backport to 4.4. If we have to backport to earlier versions, I'd prefer to add separate checks rather than moving the noop write since that changes the order of installing the config in memory and the no-op write. I'm happy to take this ticket, but I'd like to discuss it on Monday so I assigned it to backlog. |
| Comment by Pavithra Vetriselvan [ 27/Mar/20 ] |
|
Yes, I think we can close I assumed that we would be backporting both fixes since writing down the config document without doing a primary check also existed before safe reconfig. Although, I think the OplogOutOfOrder was caused by the noop entry being written on a secondary, so that seems like a more urgent fix. I am fine filing a separate ticket for the noop primary check so that it can be backported sooner. siyuan.zhou Does that sound reasonable? |
| Comment by Tess Avitabile (Inactive) [ 27/Mar/20 ] |
|
Not checking for primary before doing the noop write is a beginning-of-time bug, so we will need to backport the fix. That may need to be under a separate ticket if safe reconfig on master requires its own solution. The solution for |