Skip to content

Improve Performance#97

Open
HarishTeens wants to merge 3 commits intodscnitrourkela:developmentfrom
HarishTeens:upgrade
Open

Improve Performance#97
HarishTeens wants to merge 3 commits intodscnitrourkela:developmentfrom
HarishTeens:upgrade

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

github-actions bot commented Mar 20, 2021

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

@DesignrKnight DesignrKnight Mar 20, 2021

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

@HarishTeens HarishTeens Mar 25, 2021

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

DesignrKnight commented Mar 24, 2021

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

@github-actions
Copy link

github-actions bot commented Jul 31, 2022

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