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

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

creator-node/src/redis.js

@@ -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

creator-node/src/services/sync/processSync.js

@@ -45,19 +45,22 @@ async function processSync(

  /**
   * 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

creator-node/src/services/sync/processSync.js

@@ -510,7 +513,6 @@ async function processSync(
        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

creator-node/src/services/sync/processSync.js

@@ -527,7 +529,6 @@ async function processSync(
        )

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


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

creator-node/src/dbManager.js

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



just a naming nit for accuracy

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

creator-node/src/middlewares.js

-  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

creator-node/src/services/sync/processSync.js

creator-node/src/redis.js

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-size bot 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 5c4c6d7 Compare July 10, 2022 23:42

SidSethi changed the title ~~Content Node Global write lock~~ CON-247 Content Node Global write lock Jul 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

creator-node/src/services/sync/processSync.js Outdated Show resolved Hide resolved

creator-node/src/redis.js Outdated Show resolved Hide resolved

creator-node/src/redis.js Outdated Show resolved Hide resolved

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 master Jul 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 modal C-2632 (#3415)

0a053af

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

AudiusProject deleted a comment from linear bot Sep 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

SidSethi commented Jul 10, 2022 •

edited

Loading

SidSethi Jul 11, 2022

SidSethi Jul 11, 2022

SidSethi Jul 11, 2022

SidSethi Jul 11, 2022

SidSethi Jul 11, 2022

SidSethi Jul 12, 2022

dmanjunath left a comment

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

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

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

Conversation

SidSethi commented Jul 10, 2022 • edited Loading

Description

Tests

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

SidSethi Jul 11, 2022

Choose a reason for hiding this comment

SidSethi Jul 11, 2022

Choose a reason for hiding this comment

SidSethi Jul 11, 2022

Choose a reason for hiding this comment

SidSethi Jul 11, 2022

Choose a reason for hiding this comment

SidSethi Jul 11, 2022

Choose a reason for hiding this comment

SidSethi Jul 12, 2022

Choose a reason for hiding this comment

dmanjunath left a comment

Choose a reason for hiding this comment

SidSethi commented Jul 10, 2022 •

edited

Loading