Skip to content

Added expiration time variable #26

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

Closed
wants to merge 0 commits into from
Closed

Conversation

asierguti
Copy link

We are working on using srcache for caching data from our backend servers into our own database, sos that the frontend server can just retrieve the data from the cache.

In order to make sure that the data gets purged from time to time, and that we don't send "old" data from the cache, we wanted to add an "Expires" variable in the data header that is being cached, so that we know when we have to recache the data, and when we have to delete old data.

This patch just adds a new configuration variable in the form of "srcache_cache_valid <HTTP_STATUS> ", and a callback to analyze the expiration time from the fetched data and make sure we recache the data if the one in the cache is old enough.

@agentzh
Copy link
Member

agentzh commented Mar 17, 2014

@asierguti Thank you for the patch! Will you mind adding some corresponding test cases to the existing test suite to demonstrate how it actually works? Thanks!

@asierguti
Copy link
Author

Well, I have a couple of scripts in python.

The idea is to create a simple HTTP server which sends an HTTP answer with the current timestamp in the body.

The second script will just try to retrieve a particular URL, then wait a number of seconds, and try to grab the same URL once again. It will check if it was due to recache or not, and it will check the timestamp in the body.

I will upload both scripts shortly.

@asierguti asierguti closed this Mar 19, 2014
@agentzh
Copy link
Member

agentzh commented Mar 19, 2014

@asierguti Okay, I've had a closer look and got your idea. Basically you want to validate the response from the srcache_fetch subrequest by looking at the cached Expires response header. I agree it is useful when the cache storage backend does not support expiring itself (though using Python here is not really efficient, maybe you should just talk to elliptics with a bit of Lua using ngx_lua module's cosocket API).

Before your patch can be merged, please adjust the coding style to be consistent with the existing code base and preferably with test cases in your patch for the existing test scaffold. (BTW, I'm not interested in your Python scripts and I don't want to include any Python code in the existing test suite that is driven completely by Perl. Please look at the existing test cases for examples).

@agentzh
Copy link
Member

agentzh commented Mar 19, 2014

@asierguti Also, I think it's a good idea to introduce a srcache_fetch_validate_expires on|off directive for this new feature. It will be a waste of CPU cycles for those cache storage backends that do support expiring (like memcached and redis).

And for the automatic "Expires" header insertion part, I must say I don't like it. It is just totally wrong to always blindly override the original "Expires" response header. Though it is what you want for your particular use case but it will break others' setup.

A better approach for Expires response header insertion for your use case is to use ngx_lua module's header_filter_by_lua and the existing $srcache_expires variable exposed by ngx_srcache to insert the "Expires" response header in case of a cache miss yourself, which is much more flexible.

@asierguti
Copy link
Author

Thanks a lot for your feedback.

First of all, I will try to translate all our tests to perl. It will take
some time, since I've never programmed in perl. Hopefully, I will have it
completed by this weekend.

Regarding your suggestions, I didn't quite get what you said about
srcache_fetch_validate_expires
on|off.How is this feature supposed to work? Having our frontend servers
check the expiration dates of all the data in our backend storage servers
will increase the traffic network. Network is more important than CPU
cycles, at least for us.

Now, regarding your suggestion about LUA. To be honest, I thought about it
before. The thing is that we want to have flexibility to adjust the
expiration time according to the response code we get from our real backend
server (not the cache storage backend). For example, if we get 200, we
would like to cache the data for a week, but if we get 301, we would like
to cache it only for a day. That's why $srcache_expires
was not suitable for us. If you have any suggestion, I will be glad to
consider it.

Cheers.

On 19 March 2014 23:16, Yichun Zhang notifications@github.com wrote:

@asierguti https://github.com/asierguti Also, I think it's a good idea
to introduce a srcache_fetch_validate_expires on|off directive for this
new feature. It will be a waste of CPU cycles for those cache storage
backends that do support expiring (like memcached and redis).

And for the automatic "Expires" header insertion part, I must say I don't
like it. It is just totally wrong to always blindly override the original
"Expires" response header. Though it is what you want for your particular
use case but it will break others' setup.

A better approach for Expires response header insertion for your use case
is to use ngx_lua module's header_filter_by_lua and the existing

$srcache_expires variable exposed by ngx_srcache to insert the "Expires"
response header in case of a cache miss yourself, which is much more
flexible.

Reply to this email directly or view it on GitHubhttps://github.com//pull/26#issuecomment-38094715
.

@asierguti
Copy link
Author

Hi,

I don't know if you saw my previous comment. In case you missed it, or I did something wrong posting it, here it goes once again.


Thanks a lot for your feedback.

First of all, I will try to translate all our tests to perl. I'm in the process of doing so. I just have one small question. I would like to get in a variable the current time when testing (request part in the test), and compare it to the response. How could I do it? I can get the current time in a variable, but I don't seem to find how I can extract the "Expires" header, and compare it to the previous data.

Regarding your suggestions, I didn't quite get what you said about rcache_fetch_validate_expires on|off.How is this feature supposed to work? Having our frontend servers check the expiration dates of all the data in our backend storage servers will increase the traffic network. Network is more important than CPU cycles, at least for us.

Now, regarding your suggestion about LUA. To be honest, I thought about it before. The thing is that we want to have flexibility to adjust the expiration time according to the response code we get from our real backend server (not the cache storage backend). For example, if we get 200, we would like to cache the data for a week, but if we get 301, we would like to cache it only for a day. That's why $srcache_expires was not suitable for us. If you have any suggestion, I will be glad to consider it.

Looking forward to hearing from you.

Cheers.

@asierguti asierguti reopened this Mar 24, 2014
@agentzh
Copy link
Member

agentzh commented Mar 25, 2014

@asierguti Sorry, I'm currently busy with the company work. I'll get back to you as soon as I can manage :) Thanks!

@@ -538,15 +548,20 @@ ngx_http_srcache_test_precondition(ngx_http_request_t *r)
static void
ngx_http_srcache_post_read_body(ngx_http_request_t *r)
{
if (r == NULL) {
Copy link
Member

Choose a reason for hiding this comment

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

Style: we use 4 space indentation.

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.

2 participants