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

Feature/cache-locks #1367

Closed
wants to merge 1 commit into from
Closed

Conversation

subnetmarco
Copy link
Member

@subnetmarco subnetmarco commented Jul 8, 2016

When requesting a specific entity from the database, only one request to the database per node is allowed. This dramatically improves reliability during heavy load.

For example: let's say that 1000 req/s are being processed by a Kong node, and suddenly an in-memory entity is being invalidated and Kong needs to read it again from the database. Before this PR Kong would open 1000req/s to the database until that entity is stored into memory again. With this PR, only one read request will be opened to the database (per node), and the other requests (on the same node) will just wait until it's finally in memory again.

This also helps the performance of new nodes that are being added to the cluster (whose load goes from 0 to 100 real quick).

Closes #264.

@subnetmarco subnetmarco changed the title Feature/cache semaphore Feature/cache-locks Jul 8, 2016
local lock = resty_lock:new("cache_locks", {
exptime = 10,
timeout = 10
})
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't the lock be create on the module level instead of on each call to get_or_set?

Copy link
Member

Choose a reason for hiding this comment

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

nevermind, the lock can only lock 1 key at a time. So function level lock is required.

@mashapedeployment
Copy link
Contributor

Fixed typos.

@thibaultcha thibaultcha added pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) pr/status/needs review and removed pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) labels Jul 11, 2016
@thibaultcha
Copy link
Member

to be reopened against next

@subnetmarco subnetmarco mentioned this pull request Jul 15, 2016
@Tieske Tieske mentioned this pull request Jul 20, 2016
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