CON-247 Update redis write locking (Divergent State Recovery #1) by SidSethi · Pull Request #3415 · AudiusProject/apps

SidSethi · 2022-07-10T23:30:12Z

Description

Improve redis write locking logic

There are better ways to do write locking, but this is intended as a quick fix to unblock divergent state recovery. One longer term option is to switch to node-redlock, as documented in this linear card

improve Redis write locking logic (add retries + cleaner code)
add test coverage for redis + write locking
Remove syncLockMiddleware since it made no sense

Tests

New test coverage for Redis added
existing automated tests + maddog all pass (maddog failed in CI but same cmd succeeded locally)

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

Check logs Cannot change state of wallet

SidSethi · 2022-07-11T22:09:36Z

@@ -1,36 +1,123 @@
+/**


diff is confusing - better way to view this might be to see what's currently on master for RedisLock: https://github.com/AudiusProject/audius-protocol/blob/master/creator-node/src/redis.js
Changes:
replaces RedisLock class with WalletWriteLock object

SidSethi · 2022-07-11T22:14:15Z


 /**
 * Ensure access to each wallet, then acquire redis lock for duration of sync
+ * @notice - there's a bug where redisKey is set to the last element of `walletPublicKeys` - this code only works when `walletPublicKeys.length === 1` 🤦‍♂️


old bug - never shows up since we always call sync with single user
should fix tho lol

SidSethi · 2022-07-11T22:14:53Z

 logger.info(redisKey, 'Saved all AudiusUser entries to DB')

 await transaction.commit()
- await redisLock.removeLock(redisKey)


couldn't figure out why we have this redundant removeLock call - the one in finally block should cover

SidSethi · 2022-07-11T22:15:01Z

 )

 await transaction.rollback()
- await redisLock.removeLock(redisKey)


SidSethi · 2022-07-11T22:40:39Z

@@ -48,9 +48,9 @@ class DBManager {
 queryObj.clock = selectCNodeUserClockSubqueryLiteral



just a naming nit for accuracy

SidSethi · 2022-07-12T17:12:11Z

- req.logger.info(`syncLockMiddleware succeeded`)
- next()
-}
-


removing this since it made no sense to have - primaries would never have syncs happening at the same time

dmanjunath

overall this is much simpler than yesterday, had some questions/comments

Co-authored-by: Nikki Kang <kangaroo233@gmail.com>

[2c08c63] [C-2653] Add shallow argument to audius-query hooks (#3450) Andrew Mendelsohn [0e5d1b3] [PAY-1247] Cap height of chat textinput on mobile (#3447) Reed [efaa425] [PAY-1243] Mobile chat header touchable feedback (#3448) Reed [3980e00] [C-2647] Use entire cache to denormalize data (#3449) Andrew Mendelsohn [1395822] [PAY-1150] Disable scrolling when chat reaction popup is active (#3445) Reed [f4ad5f7] [C-2592] Fix edit playlist (#3443) Kyle Shanks [875054a] Fix mobile native change comment C-2657 (#3444) nicoback2 [63b5ab4] Bump SDK version and change walletapi to auth (#3440) nicoback2 [1611465] DMs: Use unblock/block confirmation modals on profile (#3437) Marcus Pasell [179dda6] [PAY-1142] DMs: Update chat tail (#3438) Marcus Pasell [95f1843] [PAY-946] Handle signed out users across DMs (#3423) Michael Piazza [682eebb] [C-2649] Refactor desktop collection header (#3441) Dylan Jeffers [7310fae] [PAY-800][PAY-1249][PAY-1250] Display track and playlist tiles in web chats - Part 1 (#3377) Saliou Diallo [689fbcb] [PAY-1274] DMs: Unsticky inbox unavailable message (#3435) Marcus Pasell [0212e19] Fix unread count incorrect on mobile chats (#3430) Reed [6743352] Update legacy playlist screens to use legacy inputs (#3436) Kyle Shanks [fb25e74] [CON-678] v2 user profile edit (#3431) Theo Ilie [abf827f] [PAY-1255] [PAY-1245] DMs: Add confirmation modals (#3424) Marcus Pasell [536d3c6] Make mobile chat inbox unavailable message unsticky (#3434) Reed [f11f266] [PAY-1273] Fix scrolling issues (#3433) Michael Piazza [26ec389] [PAY-1258] Add toast when chat messages are copied (#3421) Randy Schott [9ba1956] [PAY-1251] [PAY-1188] DMs: Add error message to failed message sends (#3428) Marcus Pasell [14034ae] [C-2597][C-2651] Add audius-query usage docs (#3427) Andrew Mendelsohn [16deebc] [C-2508] Update playback positions to be tracked per user id (#3374) Kyle Shanks [2d6b278] [C-2644] Fix issue where new playlist drag-drop not persisting (#3422) Dylan Jeffers [a9c2c5e] [PAY-1212] [PAY-1146] Reactions and message UI polish (#3414) Michael Piazza [ab8bd6b] Type fix for hook options (#3426) Andrew Mendelsohn [d167810] [PAY-1272] Make desktop DM text copyable (#3425) Michael Piazza [7ba5973] [PAY-925] Adds unread message icons on web (#3418) Randy Schott [645f0a5] [PAY-935] Fix ChatHeader issue (#3419) Michael Piazza [cc1c6e8] Update mobile playlist create flows to conform with new designs (#3416) Kyle Shanks [9701930] [PAY-1260] New chat tails on mobile (#3420) Reed [dd2873e] [PAY-1241] Skeleton for mobile chat list screen (#3393) Reed [a373e62] [PAY-1253] Mobile message resend button (#3413) Reed [ddc5540] Fix type error track availability modal C-2632 (#3415) nicoback2 [19929b9] [C-2601 C-2627 C-2628 C-2638] Playlist library QA (#3411) Dylan Jeffers [5a7f61a] [C-2468] Add desktop create flow (#3409) Dylan Jeffers [b0c1583] Mark chat as read on ChatScreen entry (#3410) Reed [6e120cd] [PAY-1222] Fix unread messages count (#3407) Michael Piazza [95dd306] [C-2299] Fix NowPlayingDrawer navigation deduping bug (#3405) Sebastian Klingler [01fcab4] [PAY-1195] Mobile chat screen unavailable states (#3402) Reed [67b1788] [C-2598] Add types for audius-query hooks (#3406) Andrew Mendelsohn [8bf784a] Increase space between chat reaction and copy message button (#3403) Reed [2cba881] Fix upload collection C-2630 (#3404) nicoback2 [aee7653] [PAY-1242] Mobile chats copy message button (#3399) Reed [01ec1fb] [PAY-1220] Fix chat receive bug (#3400) Michael Piazza [0ad899c] Add useAuthenticatedCallback (#3392) Dylan Jeffers [412ca92] Improve popup-menu a11y (#3394) Dylan Jeffers [ecdfb7e] [PAY-1136][PAY-1129] Add infinite loading to albums on desktop / mobile web (#3371) Randy Schott [35678eb] [PAY-1028] Fix mobile chat actions drawer touch feedback (#3388) Reed

pull-request-sizeBot added the size/L label Jul 10, 2022

global user write lock
5c4c6d7

SidSethi force-pushed the ss-global-user-writelock branch from 5bac85d to 5c4c6d7Compare July 10, 2022 23:42

SidSethi changed the title ~~Content Node Global write lock~~CON-247 Content Node Global write lockJul 11, 2022

SidSethi added 2 commits July 11, 2022 19:22

Merge branch 'master' into ss-global-user-writelock
99f28bb

Done
1694d6b

SidSethi commented Jul 11, 2022
View reviewed changes

SidSethi requested review from dmanjunath and vicky-g July 11, 2022 22:17

SidSethi marked this pull request as ready for review July 11, 2022 22:18

SidSethi assigned theoilie and jonaylor89 Jul 11, 2022

nit
5d3108c

SidSethi commented Jul 11, 2022
View reviewed changes

SidSethi removed request for dmanjunath and vicky-g July 11, 2022 23:04

lint
816d307

SidSethi mentioned this pull request Jul 12, 2022
CON-238 - primarySyncFromSecondary (Divergent State Recovery #2) #3425
Merged

SidSethi changed the title ~~CON-247 Content Node Global write lock~~CON-247 Content Node Global write lock - Divergent State Recovery (#1)Jul 12, 2022

SidSethi changed the title ~~CON-247 Content Node Global write lock - Divergent State Recovery (#1)~~CON-247 Content Node Global write lock (Divergent State Recovery #1)Jul 12, 2022

SidSethi mentioned this pull request Jul 12, 2022
CON-238 - Divergent state PR#4 - Primary Sync from Secondary #3399
Closed

Remove write lock middleware
2e005db

SidSethi commented Jul 12, 2022
View reviewed changes

SidSethi changed the title ~~CON-247 Content Node Global write lock (Divergent State Recovery #1)~~CON-247 Update redis write locking (Divergent State Recovery #1)Jul 12, 2022

SidSethi requested a review from dmanjunath July 12, 2022 17:14

dmanjunath reviewed Jul 12, 2022
View reviewed changes

Comment threadcreator-node/src/services/sync/processSync.js Outdated
Comment threadcreator-node/src/redis.js Outdated
Comment threadcreator-node/src/redis.js Outdated

Merge branch 'master' into ss-global-user-writelock
fd3a213

Remove redisKey + UserWrite
5a61327

SidSethi requested a review from dmanjunath July 12, 2022 18:24

dmanjunath approved these changes Jul 12, 2022
View reviewed changes

Merge branch 'master' into ss-global-user-writelock
4aa353a

SidSethi assigned vicky-g Jul 12, 2022

SidSethi merged commit fb3beba into masterJul 12, 2022

SidSethi deleted the ss-global-user-writelock branch July 12, 2022 22:08

SidSethi mentioned this pull request Jul 13, 2022
Fix Redis circular dependency #3441
Merged

sliptype pushed a commit that referenced this pull request Sep 10, 2023
Fix type error track availability modalC-2632(#3415)
0a053af
Co-authored-by: Nikki Kang <kangaroo233@gmail.com>

AudiusProject deleted a comment from linearBotSep 11, 2023

Navigation Menu

Uh oh!

CON-247 Update redis write locking (Divergent State Recovery #1) - #3415

CON-247 Update redis write locking (Divergent State Recovery #1)#3415
SidSethi merged 9 commits into
masterfrom
ss-global-user-writelock

SidSethi commented Jul 10, 2022•
edited
Loading

Uh oh!

SidSethiJul 11, 2022

Uh oh!

SidSethiJul 11, 2022

Uh oh!

SidSethiJul 11, 2022

Uh oh!

SidSethiJul 11, 2022

Uh oh!

SidSethiJul 11, 2022

Uh oh!

SidSethiJul 12, 2022

Uh oh!

dmanjunath left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		@@ -48,9 +48,9 @@ class DBManager {
		queryObj.clock = selectCNodeUserClockSubqueryLiteral

Uh oh!

Conversation

SidSethi commented Jul 10, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

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

Uh oh!

SidSethiJul 11, 2022

Choose a reason for hiding this comment

Uh oh!

SidSethiJul 11, 2022

Choose a reason for hiding this comment

Uh oh!

SidSethiJul 11, 2022

Choose a reason for hiding this comment

Uh oh!

SidSethiJul 11, 2022

Choose a reason for hiding this comment

Uh oh!

SidSethiJul 11, 2022

Choose a reason for hiding this comment

Uh oh!

SidSethiJul 12, 2022

Choose a reason for hiding this comment

Uh oh!

dmanjunath left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SidSethi commented Jul 10, 2022•
edited
Loading