Prevent extra unnecessary reconfig events

[vicky-g]

Description

There were unnecessary reconfig events being made. To prevent this from happening, we have to pass in the old replica set and call the fn that requires passing in the old replica set.

Tests

1. Created user
2. Uploaded 2 tracks
3. Checked existing replica set
4. Stopped a secondary
5. Waited to see if user got rolled onto another secondary
6. Confirmed that one of user's replica set nodes successfully rolled onto another secondary

{"name":"audius_creator_node","hostname":"baf7882f9f52","pid":26576,"queue":"update-replica-set-queue","jobId":"2","level":30,"msg":"[_issueUpdateReplicaSetOp] Reconfig SUCCESS: userId=2 wallet=0x3ce45ff75ed2e9e8439dbc57e835bc37bcedd0a2 old replica set=[http://cn2_creator-node_1:4001,http://cn3_creator-node_1:4002,http://cn4_creator-node_1:4003] | new replica set=[http://cn2_creator-node_1:4001,http://cn3_creator-node_1:4002,http://cn1_creator-node_1:4000] | reconfig type=[ONE_SECONDARY]","time":"2022-08-10T21:51:49.014Z","v":0,"logLevel":"info"}

7. Confirmed that the other user's replica set node did not roll onto another secondary

{"name":"audius_creator_node","hostname":"7af26083da6e","pid":10184,"queue":"update-replica-set-queue","jobId":"1","level":30,"msg":"[_issueUpdateReplicaSetOp] skipping updateReplicaSet as reconfig already occurred for userId=2 wallet=0x3ce45ff75ed2e9e8439dbc57e835bc37bcedd0a2","time":"2022-08-10T21:52:27.790Z","v":0,"logLevel":"info"}

Monitoring - How will this change be monitored? Are there sufficient logs / alerts?

See logs surrounding _issueUpdateReplicaSetOp.

Followups

• can we make this smarter than letting the txn fail? we will see 5 retries that are useless. maybe we can also instead check the replica set on chain instead of making an extra reconfig call

[dmanjunath]

Great catch! Looks good to me but will let @SidSethi give the ✅ as well because i'm not as familiar with this flow as he is

[SidSethi]

huge find 💪
few small things, otherwise looks great

[SidSethi]

nit

const {
  primary: oldPrimarySpId,
  secondary1: oldSecondary1SpId,
  secondary2: oldSecondary2SpId
} = ContentNodeInfoManager.getCNodeEndpointToSpIdMap()

[vicky-g]

I don't think we can do this? getCNodeEndpointToSpIdMap returns an object with keys as the endpoints

[SidSethi]

oh yeah my b, i meant this

const {
  [primary]: oldPrimarySpId,
  [secondary1]: oldSecondary1SpId,
  [secondary2]: oldSecondary2SpId
} = ContentNodeInfoManager.getCNodeEndpointToSpIdMap()

also like, this is a nit haha

[vicky-g]

oh wow lol I didnt know this was possible! ok ill update it

[SidSethi]

oh yeah its dope

[SidSethi]

nit can we make this logger.warn or .error for easier loggly filtering

[vicky-g]

hm I feel like this shouldn't be a warn or error log though? like if this update reconfig happens successfully, then all is well? we should be filtering by the log instead?

[SidSethi]

whoops i meant to put this comment on line 527

logger.info(
          `[_issueUpdateReplicaSetOp] skipping _updateReplicaSet as reconfig already occurred for userId=${userId} wallet=${wallet}`
        )

[SidSethi]

idk if blocked on my re-approval but this is good to go, all my comments are your call, can merge either way