[SERVER-48733] Update expectedPrimaryNodeId parameter in awaitNodesAgreeOnPrimary Created: 11/Jun/20  Updated: 29/Oct/23  Resolved: 16/Jul/20

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 4.7.0

Type: Bug Priority: Major - P3
Reporter: Ali Mir Assignee: Ali Mir
Resolution: Fixed Votes: 0
Labels: neweng
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Repl 2020-07-13, Repl 2020-07-27
Participants:
Linked BF Score: 18

 Description   

In the function awaitNodesAgreeOnPrimary in replsettest.js, we pass in a parameter expectedPrimaryNodeId. This refers to the node we expect to become primary. However, we use this parameter to index into the list of nodes passed in, and so it is misleading to name this variable as the nodeId when in fact it is the index of the expected primary in the nodes list parameter. Here is an instance in the codebase where we are misusing this parameter. We try to pass in 1 as the expectedPrimaryNodeId, as we expect node1 to become primary (shown in the previous line), but because we index into the passed in nodes list, we set the expected primary to be node 2. 

A possible solution to this problem would be to change the parameter in awaitNodesAgreeOnPrimary to actually take in a unique nodeId. There are only a few places in the codebase where we use this third parameter, so changing it should cause minimal disruption to existing tests.



 Comments   
Comment by Githook User [ 16/Jul/20 ]

Author:

{'name': 'Ali Mir', 'email': 'ali.mir@mongodb.com', 'username': 'ali-mir'}

Message: SERVER-48733 Update expectedPrimaryNodeId parameter in awaitNodesAgreeOnPrimary
Branch: master
https://github.com/mongodb/mongo/commit/ee7c632b25d56f2d7c47edeeb3a81ef206f61924

Comment by Ali Mir [ 15/Jun/20 ]

siyuan.zhou Done.

Comment by Siyuan Zhou [ 15/Jun/20 ]

As discussed in triage meeting, the issue isn't using node id as node index, but using the node id as the index in the given nodes arguments. The proposed solution still makes sense though.

ali.mir, could you please link the BF ticket to this? This issue isn't the root cause of the BF, so I don't see the necessity of a quick fix on the misleading usage as long as it doesn't cause failures.

Comment by Judah Schvimer [ 11/Jun/20 ]

ReplSetTest refers to node indexes as nodeId pretty frequently unfortunately, we may want a more consistent fix, or to just fix the one place that's wrong.

Generated at Thu Feb 08 05:17:56 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.