Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve StateMachineQueue #3052

Merged
merged 5 commits into from May 9, 2022
Merged

Conversation

theoilie
Copy link
Contributor

@theoilie theoilie commented May 6, 2022

Description

Tests

Searched local logs to make sure Bull events are being logged, jobIds are being logged, and processStateMachineOperation is running as expected.

How will this change be monitored? Are there sufficient logs?

Search logs for "processStateMachineOperation() Decision Tree" and see if there are any gaps in any node. Debug gaps by looking for stateMachineQueue Job [Error|Waiting|Active|Lock Extension Failed|Completed|Stalled|Failed] logs.

Copy link
Contributor

@SidSethi SidSethi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!
if you've confirmed this works locally, prob good to take out to single staging node from branch before merging, esp with the bull major version bump. that one is scary

side note - i noticed you didn't bring over the Queue#obliterate() step. totally fine, just flagging that may be another lever we can pull
see docs for info - https://www.notion.so/audiusproject/Bull-Queue-6bef860a5e784d19a099df8c32003fdf#b0bc0f63fb754a33a08607a22e628e21

creator-node/package.json Show resolved Hide resolved
creator-node/src/snapbackSM/snapbackSM.js Show resolved Hide resolved
creator-node/src/snapbackSM/snapbackSM.js Show resolved Hide resolved
@theoilie
Copy link
Contributor Author

theoilie commented May 6, 2022

lgtm! if you've confirmed this works locally, prob good to take out to single staging node from branch before merging, esp with the bull major version bump. that one is scary

side note - i noticed you didn't bring over the Queue#obliterate() step. totally fine, just flagging that may be another lever we can pull see docs for info - https://www.notion.so/audiusproject/Bull-Queue-6bef860a5e784d19a099df8c32003fdf#b0bc0f63fb754a33a08607a22e628e21

great! I'll see how everything looks on a single node.

I left out the obliterate because it seemed like something that might do better in an experiment instead of on all nodes. I also figured if we already removeOnComplete and removeOnFail then I'm not sure how much it would help to obliterate once when the CN initializes

@theoilie theoilie force-pushed the theo-statemachinequeue-improvements branch from 9ae7a75 to eb8c26d Compare May 6, 2022 20:27
@theoilie theoilie force-pushed the theo-statemachinequeue-improvements branch from fe375ca to e1f7cfb Compare May 6, 2022 21:50
@theoilie theoilie force-pushed the theo-statemachinequeue-improvements branch from e989768 to 3f5da6e Compare May 6, 2022 22:35
@theoilie
Copy link
Contributor Author

theoilie commented May 6, 2022

I noticed during one test on staging that the delayed job (with id 2217) ran, but the repeatable job never ran. this could be a state issue from restarting a few times, so I added back Queue#obliterate to see if that helps. I looked into it more and it doesn't seem like it'll hurt anything (also Vicky said it helped with a Bull issue she had before)

@theoilie
Copy link
Contributor Author

theoilie commented May 9, 2022

soaking these changes on staging over the weekend didn't fix the Bull issues, but the increased logging helped and there were no new regressions. re-requesting a review

@theoilie theoilie requested a review from SidSethi May 9, 2022 13:48
Copy link
Contributor

@SidSethi SidSethi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with major version bump, lets both go through logs for other CN queues and ensure proper function

creator-node/src/snapbackSM/snapbackSM.js Show resolved Hide resolved
creator-node/src/snapbackSM/snapbackSM.js Outdated Show resolved Hide resolved
Copy link
Contributor

@SidSethi SidSethi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm pending version bump confirmation

@theoilie theoilie merged commit 14b2fb0 into master May 9, 2022
@theoilie theoilie deleted the theo-statemachinequeue-improvements branch May 9, 2022 17:06
sliptype pushed a commit that referenced this pull request Sep 10, 2023
[e804aa1] [C-2248, C-2373] Use playlistUpdates, remove legacyNotifications (#3094) Dylan Jeffers
[824933e] [C-2366] Improve web notification selection performance (#3103) Dylan Jeffers
[4b8edef] [PLAT-696] Add trending-playlists/underground notifications (#3089) Dylan Jeffers
[1f9cf3e] [C-2275] Fix android drawer offsets (#3095) Dylan Jeffers
[fc14c82] [PAY-1063][PAY-1085][PAY-1086] Update UI for inaccessible gated tracks from favorites and history pages (#3100) Saliou Diallo
[b0441f5] [C-2365] Update play buttons on web and mobile to show resume when track is current (#3101) Kyle Shanks
[453910f] [C-2378] Add upload v2 feature flag (#3099) Sebastian Klingler
[962a6df] [C-2337] Remove reachability mobile web (#3090) Raymond Jacobson
[4ad5cd2] Fix visible collectibles for upload popup (#3093) Saliou Diallo
[c143078] Fix feature flag bug (#3092) Saliou Diallo
[44435b5] Fix upload prompt modal learn more url (#3091) Saliou Diallo
[c9024ad] Use chat.messagesStatus instead of selector (#3087) Reed
[38d43c4] [C-2369] Fix issue where notification poll can break app on signout (#3088) Dylan Jeffers
[90122d9] [PAY-923] DMs: Add desktop entrypoints (#3083) Marcus Pasell
[00f27e8] [PAY-907] Mobile chat reactions (#3020) Reed
[4678b89] DMs: Fix broken typecheck on main (#3086) Marcus Pasell
[756ade4] [PAY-1000][PAY-1084][PAY-1096][PAY-1097][PAY-1098] - More gated content fixes (#3085) Saliou Diallo
[820aa9d] Fix upload and repost probers tests and lint (#3076) Sebastian Klingler
[345607e] [C-2320] Fix profile socials alignment (#3079) Dylan Jeffers
[569199c] Fix prod build timeout (#3084) Sebastian Klingler
[12f6c22] Remove ports for local dev (#3082) Theo Ilie
[1940618] Fix broken Main build due to typeerror (#3080) Marcus Pasell
[eb8d47e] [PAY-1082] DMs: Dedupe sent messages (#3066) Marcus Pasell
[50a11c3] Update SDK to 2.0.3-beta.0 (#3078) Marcus Pasell
[c420fbb] Clean up NPM package lock (#3077) Marcus Pasell
[35d1124] [C-2327] Add playlist updates slice (#3063) Dylan Jeffers
[59862ad] [C-2344] Update the web playbar scrubber to respect the playback speed of podcasts (#3075) Kyle Shanks
[ffeb0d3] [C-2349] Default download on wifi only to false (#3074) Andrew Mendelsohn
[cafae41] [C-2325] Fix playlist table date-added column (#3073) Dylan Jeffers
[384a510] [PAY-927] DMs: Empty messages state (#3068) Marcus Pasell
[1132f83] Update @jup-ag/core to 2.0.0-beta.9 (#3072) Marcus Pasell
[49c0ebf] [PAY-1072] Change "Download App" icon on Settings Page (#3067) Marcus Pasell
[928dcaf] [PAY-1056] - More gated content updates and fixes (#3070) Saliou Diallo
[1e1f769] [C-2345] Move PlaybackRate drawer to common drawers map (#3071) Kyle Shanks
[f5d1251] Fix web-dist CI steps (#3069) Sebastian Klingler
[5f89800] Fix heavy rotation playlist on client (#3056) sabrina-kiam
[c0191e2] [C-2316] Add remote config for all oauth verification (#3052) Raymond Jacobson
[40f5627] [PAY-1074][PAY-1075][PAY-1076][PAY-1080] - Update availability settings states + more QA fixes (#3059) Saliou Diallo
[5be60ac] [C-2339] Update podcast control updates to also work for audiobooks (#3065) Kyle Shanks
[163ebf5] [C-2297] Add fallback flag to podcast feature (#3064) Sebastian Klingler
[f206391] [PAY-904] - Add gated content upload prompt (#3057) Saliou Diallo
[1afc4e5] [C-1344] Move probers to monorepo and make tests pass (#3061) Sebastian Klingler
[e198279] Remove random line (#3062) Saliou Diallo
[24a001b] Add playback position logic for mobile (#3051) Kyle Shanks
[d210124] [PAY-1070] Update TabSlider/SegmentedControl slider size on resize (#3044) Marcus Pasell
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants