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

[CON-213] - Paginate delist adding and small refactor #3287

Merged
merged 24 commits into from
Jun 23, 2022

Conversation

vicky-g
Copy link
Contributor

@vicky-g vicky-g commented Jun 21, 2022

Description

Purpose: To prevent OOM errors on delist init.
New behavior: This PR is to paginate adding content to the delist init + remove an unused part of the adding (bug discovered on paginating the add).

See design doc for more info

Tests

Automated:
There is no net new functionality, just batching, removing a flow that is not used, and adding retries.

Manual:

  • Deployed this branch hash to stage content node 5 to observe if the ipfs route still works as expected and it does.
  • The time spent in init'ing the class is ~77001ms (1min 17s), with batch size of 200

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

The CN should not come up if the initialization fails to add content. And the content node will print out the init err if the init does fail.

We have some retries here with the log addAggregateCIDsToRedis to tell us if the app fails to come up.

@gitguardian
Copy link

gitguardian bot commented Jun 21, 2022

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id Secret Commit Filename
3509188 Generic High Entropy Secret 054316a .circleci/config.yml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@vicky-g vicky-g requested a review from dmanjunath June 21, 2022 19:59
@vicky-g vicky-g marked this pull request as ready for review June 21, 2022 20:00
@vicky-g vicky-g added the content-node Content Node (previously known as Creator Node) label Jun 21, 2022
Copy link
Contributor

@dmanjunath dmanjunath left a comment

Choose a reason for hiding this comment

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

pagination and cleanup of REDIS_MAP_BLACKLIST_SEGMENTCID_TO_TRACKID_KEY look great. biggest question i have is does the nginx cache have something to do with this PR or is that just an unrelated change? also we should put back any tests that have been skipped

creator-node/compose/docker-compose.yml Outdated Show resolved Hide resolved
creator-node/compose/env/shellEnv1.sh Outdated Show resolved Hide resolved
creator-node/test/lib/blacklistManager.js Show resolved Hide resolved
creator-node/src/blacklistManager.js Outdated Show resolved Hide resolved
@dmanjunath
Copy link
Contributor

@vicky-g this is with prod db and disk state?

  • The time spent in init'ing the class is ~77001ms (1min 17s), with batch size of 200

@vicky-g
Copy link
Contributor Author

vicky-g commented Jun 22, 2022

@vicky-g this is with prod db and disk state?

  • The time spent in init'ing the class is ~77001ms (1min 17s), with batch size of 200

no, this is just with the db state. the disk was not copied over, but it shouldnt matter since the delisting initialization does not touch things on disk

@vicky-g vicky-g requested a review from dmanjunath June 22, 2022 17:25
Copy link
Contributor

@dmanjunath dmanjunath left a comment

Choose a reason for hiding this comment

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

looks good! left one more question but not blocking, if you think we need async retry go ahead and merge

@vicky-g vicky-g merged commit e750d7d into master Jun 23, 2022
@vicky-g vicky-g deleted the vg-paginate-bl-add-and-refactor branch June 23, 2022 15:22
raymondjacobson pushed a commit that referenced this pull request Jun 25, 2022
* logging memory used

* batch 500 + logs hm..

* quick exit for easy testing

* remove unused fn

* dont kill app after blacklist init

* remove test fns

* update duration log

* move init flag

* no good bye msg lol

* add async retry

* remove early exit

* extra new line

* fix log

* remove deleting non-existent keys

* update logs

* removing timing responsibiltiy from bl

* add completion log

* update var name + set enabled to true

* Revert "update var name + set enabled to true"

This reverts commit f30efb8.

* remove skips

* remove async retry

* remove unused var
raymondjacobson pushed a commit that referenced this pull request Jun 25, 2022
* logging memory used

* batch 500 + logs hm..

* quick exit for easy testing

* remove unused fn

* dont kill app after blacklist init

* remove test fns

* update duration log

* move init flag

* no good bye msg lol

* add async retry

* remove early exit

* extra new line

* fix log

* remove deleting non-existent keys

* update logs

* removing timing responsibiltiy from bl

* add completion log

* update var name + set enabled to true

* Revert "update var name + set enabled to true"

This reverts commit f30efb8.

* remove skips

* remove async retry

* remove unused var
sliptype pushed a commit that referenced this pull request Sep 10, 2023
[eaaf509] [C-2565] Improve mobile app-redirect-popover experience (#3344) Dylan Jeffers
[1755765] Update sdk to 2.0.3-beta.18 (#3345) Sebastian Klingler
[571c991] [CON-676] Support storage v2 playlists+albums, and bump libs version to 2.0.3-beta.17 (#3314) Theo Ilie
[3cca778] [C-2577] Fix mobile web social buttons by removing pull-to-refresh (#3343) Dylan Jeffers
[0915ff4] [C-2519] Ignore TOS deeplink (#3341) Dylan Jeffers
[39fda1a] Feature flag gates to send metadata directly through chain (#3311) Michelle Brier
[80278ce] Upgrade sdk to beta.16, update install script (#3339) Dylan Jeffers
[7c3aaa5] [C-2451] Fix infinite buffering in audio player (#3336) Sebastian Klingler
[be68f2e] Prevent fetchBlockees/blockers when chat disabled (#3338) Dylan Jeffers
[779cfa5] [C-2568] Silence player/error when audio is loaded (#3330) Sebastian Klingler
[869504c] HOTFIX track notification navigation (#3335) Dylan Jeffers
[f30507c] Dismiss keyboard on mobile chat screen top right press (#3333) Reed
[ad7938c] DMs: Don't increment unread if it's your own message (#3331) Marcus Pasell
[565f182] Fix keyboard occluding textinput in mobile chat screen (#3332) Reed
[a82cf59] DMs: Fix unread indicator not going away (#3329) Marcus Pasell
[e47f323] Mobile chat housekeeping (#3288) Reed
[47a5e84] [PAY-1207] Use Followers instead of Mutuals on compose chat modal (#3323) Marcus Pasell
[03c587d] [C-2393] Cache pods in CI (#3317) Sebastian Klingler
[22f4d46] Remove identity subscriptions reads/writes (#3005) Michelle Brier
[b9ddae1] [C-2566] Prevent player error on skip (#3327) Dylan Jeffers
[ca246f9] DMs: Fix store bug sending messages (#3326) Marcus Pasell
[309af58] Update fetchPermission condition (#3324) Dylan Jeffers
[a8e301e] [PAY-1199] Mobile inbox settings functional (#3290) Reed
[adc7854] [C-2563] Fix demo build (#3322) Sebastian Klingler
[d703f6e] Fix search-results saga error due to yield* (#3321) Dylan Jeffers
[d940ae6] Fix internal/external audius urls (#3320) Dylan Jeffers
[96cd42f] [C-2560] Improve checking for new notifs (#3316) Dylan Jeffers
[3130546] I broke main and I'm sorry (#3319) Marcus Pasell
[a3062fe] Remove dependency on `download.cid` for determining download eligibility (#3307) Michael Piazza
[fe04046] [PAY-901] hotfix: Use SPL amounts for BuyAudio flow (#3318) Marcus Pasell
[1c94f06] Fix mobile prem content track headers (#3315) Reed
[f87cb04] [PAY-1108] Chats Screen avoids play bar (#3295) Reed
[c5450a0] Revert "[PAY-901] hotfix: TransactionDetailsModal now expects wei amounts" (#3313) Marcus Pasell
[e3ca9e3] Fix stylelint (#3312) Raymond Jacobson
[2031f1a] Debounce reaction for longer on web (#3305) Raymond Jacobson
[492fa67] Notification settings improvements (#3264) Michelle Brier
[59710d6] Rename sdk_v2 to sdk_discovery_node_selector (#3310) Dylan Jeffers
[8519910] [PLAT-863] Fix reaction endpoint when sdk-v2 (#3309) Dylan Jeffers
[00ba722] Fix index issue with constructUrl (#3308) Dylan Jeffers
[b0b32ff] [PLAT-904] Fix duplicate notifications (#3304) Dylan Jeffers
[14c7814] Pass metadata to libs for addTracksToChainAndCnode (#3285) Michelle Brier
[75456fd] Add search multimap (#3306) Raymond Jacobson
[f8f5f02] [C-2282] Show tracks tab on own profile if all tracks hidden (#3294) Andrew Mendelsohn
[49fc834] [PAY-901] hotfix: TransactionDetailsModal now expects wei amounts (#3297) Marcus Pasell
[f1366ac] Fix eager fetch (#3302) Raymond Jacobson
[4b639ba] [C-2542] Update delete playlist confirmation drawer to conform to new designs (#3303) Kyle Shanks
[1ac5ec2] Fix taste maker copy (#3301) Isaac Solo
[de48e11] [PAY-1178] Fix new messages on new/unloaded chats (#3298) Marcus Pasell
[ea6ed55] Redirect if not allowed ai (#3300) Raymond Jacobson
[985a05c] Fix ai attrib upload (#3299) Raymond Jacobson
[a586435] [C-2554] Add AI Attribution (#3296) Dylan Jeffers
[64dde7f] [CON-674] Make track edit use v2 flow for storage v2 tracks (#3289) Theo Ilie
[d38d483] [C-2543, C-2544] Initial updates to mobile playlist management (#3287) Kyle Shanks
[ec9dc0e] Fix devnet in wallet authorize for saga phone (#3291) Raymond Jacobson
@AudiusProject AudiusProject deleted a comment from linear bot Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content-node Content Node (previously known as Creator Node) size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants