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

[PM-5111] Reduce calls to config endpoint #7069

Merged
merged 2 commits into from
Dec 6, 2023

Conversation

trmartin4
Copy link
Member

@trmartin4 trmartin4 commented Dec 1, 2023

Type of change

- [ ] Bug fix
- [ ] New feature development
- [X] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

We are currently making ~70k calls/minute to the /config endpoint. This compares to around ~6k/minute for sync, for comparison. In testing, I've identified some ways to reduce these calls incrementally.

  • There appeared to be a duplicate nexting of the activeAccount$ observable on account change. The setActiveAccount and addAccount were both issuing the change.
  • I added distinctUntilChanged to the subscription on the EnvironmentService, so that if the activeAccount$ were to next the same user ID it will not re-retrieve the environment URLs. The assumption here is that a given user will have the same environment URLs when changing accounts.

Code changes

  • environment.service.ts: Added distinctUntilChanged
  • state.service.ts: Removed extra observable next on account change

Before you submit

  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team
  • Ensure that all UI additions follow WCAG AA requirements

@github-actions github-actions bot added the needs-qa Marks a PR as requiring QA approval label Dec 1, 2023
@trmartin4 trmartin4 changed the title Removed second active account observable and added distinctUntilChanged. Reduce calls to config endpoint Dec 1, 2023
@trmartin4 trmartin4 marked this pull request as ready for review December 1, 2023 20:16
@trmartin4 trmartin4 requested a review from a team as a code owner December 1, 2023 20:16
@trmartin4 trmartin4 requested a review from withinfocus December 1, 2023 20:17
@trmartin4 trmartin4 changed the title Reduce calls to config endpoint [PM-5077] Reduce calls to config endpoint Dec 1, 2023
@trmartin4 trmartin4 changed the title [PM-5077] Reduce calls to config endpoint [PM-5011] Reduce calls to config endpoint Dec 1, 2023
@trmartin4 trmartin4 changed the title [PM-5011] Reduce calls to config endpoint [PM-5111] Reduce calls to config endpoint Dec 1, 2023
@bitwarden-bot
Copy link

bitwarden-bot commented Dec 2, 2023

Logo
Checkmarx One – Scan Summary & Details349c3d65-d962-4a34-a0de-c78f20f02ea5

Fixed Issues

Severity Issue Source File / Package
LOW Use_Of_Hardcoded_Password /apps/web/src/app/admin-console/organizations/members/services/account-recovery/account-recovery.service.spec.ts: 112
LOW Use_Of_Hardcoded_Password /apps/web/src/app/admin-console/organizations/members/services/account-recovery/account-recovery.service.spec.ts: 112

@trmartin4 trmartin4 removed the needs-qa Marks a PR as requiring QA approval label Dec 6, 2023
@trmartin4 trmartin4 merged commit 299a880 into master Dec 6, 2023
55 of 59 checks passed
@trmartin4 trmartin4 deleted the auth/pm-5077/reduce-config-endpoint-calls branch December 6, 2023 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants