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

Add global lru for routes with keys being the appname + path #472

Merged
merged 5 commits into from
Jan 5, 2017

Conversation

seiflotfy
Copy link
Contributor

Use a global LRU for routes matching during function calls

Copy link
Contributor

@ucirello ucirello left a comment

Choose a reason for hiding this comment

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

This shall be OK for single-node deployments. On a multi-node deployment, we need somehow to be able to invalidate local node's cache whenever the DB gets updated by some other entity/node.

@@ -5,4 +5,8 @@ Copyright 2013 Julien Schmidt. All rights reserved. BSD-license

For: api/server/internal/routecache/lru.go
For: api/server/singleflight.go
Copyright 2012 Google Inc. All rights reserved. Apache 2 license
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the duplication here?

return
}

// Delete ...
Copy link
Contributor

Choose a reason for hiding this comment

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

function description is missing.

@@ -99,5 +99,8 @@ func (s *Server) handleRouteCreate(c *gin.Context) {
return
}

// For any create just push the new route to the front of the list
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment really necessary? it seems quite self-evident for me that Refresh will push the new route to the front of the list.

For: api/server/singleflight.go
Copy link
Contributor

Choose a reason for hiding this comment

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

Still duplicated...

@ucirello ucirello merged commit 6f8e946 into master Jan 5, 2017
@ucirello ucirello deleted the globallru branch January 5, 2017 18:08
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.

3 participants