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.

Sign up for GitHub

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

Jump to bottom

Improve Performance #97

Open
wants to merge 3 commits into
base: development
Choose a base branch
from
Open

Improve Performance #97

wants to merge 3 commits into from

Conversation

HarishTeens
Copy link
Member

@HarishTeens HarishTeens commented Mar 20, 2021 •
edited
Loading

This PR includes

  • Use node-cache to cache the decoded JWT which doesn't make firebase calls until the JWT expires whose TTL is set to 36000 which is the TTL of a Firebase JWT token
  • Make callbacks asynchronously to reduce wait time on the server side.

Some Metrics

Operation Prev Time Cur Time
authUser 8-10s 600-700ms
JWT decode(on hit) 300ms <1ms

@HarishTeens
cache jwt token
ef45294 
every time a jwt is to be decoded, check in the node cache instead of decoding it everytime using the verifyIdToken method
@HarishTeens
improve performance of authFlow
3e17054 
Make unnecessary callbacks async to give a response to the client asap.
Use lean and select mongoose functions
@github-actions
Copy link

image image
Hello @HarishTeens , That's a great improvement to the code. Have a pinch of paitence while the reviewer gets impressed by the changes you made. Here are some doggos for company while you are waiting for the merge and marching ahead with your Hacktoberfest Contributions, Check your Dashboard for more information on Hacktoberfest. Stay safe 🚀 .

DesignrKnight
DesignrKnight reviewed Mar 20, 2021
View reviewed changes
sources/apollo/server.js Outdated
Comment on lines 47 to 56
try {
console.time("firebase");

let userToken = tokenCache.get(idToken);
if (userToken === undefined) {
const decodedToken = await firebaseApp.auth().verifyIdToken(idToken)
// eslint-disable-next-line prefer-destructuring
userToken={ uid: decodedToken.uid, mongoID: decodedToken.mongoID }
tokenCache.set(idToken, userToken,36000);
}
Copy link
Member

Choose a reason for hiding this comment

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

Scenario
at t=0, Firebase minted JWT for the user
at t=30 min, App sent that token to the server
This caches the JWT for 60 mins, whereas it should cache only for 30 mins

Ideally
Store the exp of the JWT when you cache and check it also

Copy link
Member Author

Choose a reason for hiding this comment

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

Great Suggestion. Will do it soon! 🚀

@DesignrKnight
Copy link
Member

Deployed to staging at http://staging.nitrkl.in/

@github-actions
Copy link

image image
Hello @abhibhaw , That's a great improvement to the code. Have a pinch of paitence while the reviewer gets impressed by the changes you made. Here are some doggos for company while you are waiting for the merge and marching ahead with your Hacktoberfest Contributions, Check your Dashboard for more information on Hacktoberfest. Stay safe 🚀 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DesignrKnight DesignrKnight DesignrKnight left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@HarishTeens @DesignrKnight @abhibhaw