Use my fork as base for this

[onnimonni]

Hey @agentzh!

This issue is result of the conversation we had in: https://github.com/openresty/srcache-nginx-module/issues/60

I prepared a fork from the original work of Sergey A. Osokin <osa@FreeBSD.ORG.ru> from: https://people.freebsd.org/~osa/ngx_http_redis-0.3.8.tar.gz into my github account: https://github.com/onnimonni/redis-nginx-module. It should be ready to be forked here.

~Also one more question: What do you think about changes in:~ ~https://github.com/Yongke/ngx_http_redis-0.3.7/commit/9075b7d20c2881ccaf367319904b1490922fbf10~ ~I didn't yet port those changes. Are they useful? Should those be ported here too?~

I ported all of the changes from Yongke and enabled tests with docker and travis.

[onnimonni]

I managed to get the test depencies into order and the tests are now running. Currently this works just fine when redis contains some data but the nginx locks if redis returns (nil).

[onnimonni]

Now the tests are working too. Figuring out how to install the dependencies for the tests was quite painful but now we have quite good test coverage.

[onnimonni]

I'm not sure why but this fork works just fine with nginx version:

1.9.15
1.11.2

But not with:

1.11.13
1.12.0 (Current Stable)
1.13.2 (Current Mainline)

They all shout that:

/build//ngx_http_redis_module.c: In function 'ngx_http_redis_upstream_add':
/build//ngx_http_redis_module.c:1065:21: error: 'ngx_http_upstream_srv_conf_t {aka struct ngx_http_upstream_srv_conf_s}' has no member named 'default_port'
         if (uscfp[i]->default_port
                     ^~
/build//ngx_http_redis_module.c:1067:24: error: 'ngx_http_upstream_srv_conf_t {aka struct ngx_http_upstream_srv_conf_s}' has no member named 'default_port'
             && uscfp[i]->default_port != url->default_port)
                        ^~
make[1]: *** [objs/Makefile:1167: objs/addon/build/ngx_http_redis_module.o] Error 1

[onnimonni]

I managed to fix them by using fixes from PR: https://github.com/openresty/redis2-nginx-module/pull/42.

Now the tests are passing for everything from 1.13.2->1.0.15.

This should be good to go but of course I'm waiting for your review.

[agentzh]

@onnimonni Thanks for your hard work on this. Maybe we should migrate from nginx's Test::Nginx to our openresty's Test::Nginx test scaffold, just like ngx_redis2's test suite? This way we can also easily enjoy all the advanced testing features provided by our Test::Nginx test scaffold, as documented here:

https://openresty.gitbooks.io/programming-openresty/content/testing/

And also integrate it into our EC2 test cluster here:

https://qa.openresty.org/

Just my 2 cents.

[onnimonni]

Sounds good, I'll check these soon too.

[onnimonni]

I agree that the tests would be more prettier using openresty Test::Nginx scaffold but after some research it would require major refactoring of the tests.

For example this is is more easier with ngx_redis2 because you can also write data to redis with that module. With ngx_http_redis it is not possible to write data to redis so the redis needs to be populated with alternative methods.

The openresty Test::Nginx doesn't provide helpers for running one time daemons like the current toolset does.

Maybe you just know a better way to achieve this? How would you for example populate the redis server in tests?

And I think that the ngx_redis2 only tests against one static redis instance? In this case the tests should run against different redis instances so we can test with AUTH and without AUTH.

[onnimonni]

I wanted to have the full version history from the earlier ngx_http_redis versions so I crawled them from:

https://people.freebsd.org/~osa/ngx_http_redis-X.X.X.tar.gz

and included them to:

~https://github.com/onnimonni/redis-nginx-module-with-history~ => https://github.com/onnimonni/redis-nginx-module

Now the repo reflects the age and development of this module better.

I also refactored the tests to use openresty Test::Nginx but they still need a custom redis instance.

I hope this is good enough now.

[agentzh]

@onnimonni Well, you can use lua-resty-redis or ngx_redis2 in the test suite to populate data. It's fine for the test suite to depend on external nginx modules or lua libraries.

[onnimonni]

Ah thats a good idea :)! Thanks for the input!

I think openresty doesn't currently have an AUTH enabled redis server for tests. Can we add an additional AUTH enabled redis server into: https://qa.openresty.org/?

I know you are quite busy and I really appreciate all you have done for this so far. If you want I can help you to setup this in the QA server.

If you do that I could add few additional tests into redis2_nginx_module for testing with AUTH too.

If it is installed I could use it from the tests with env like these two: $ENV{TEST_NGINX_REDIS_AUTH_ENABLED_PORT} (This could be running in 6380 for example) $ENV{TEST_NGINX_REDIS_AUTH_ENABLED_PASS} (This is the input for requirepass)

[onnimonni]

I removed the code which starts custom redis instance for every test and the tests now rely on redis database to be available in 127.0.01.

I didn't use lua-resty-redis or ngx_redis2 because I wanted to keep this simple.

In order to use the tests the system running them now needs to have:

• redis database running in 127.0.0.1:$TEST_NGINX_REDIS_PORT (default is 6379).
• another redis database with AUTH password $TEST_NGINX_REDIS_AUTH_ENABLED_PASS enabled (default: password) running in 127.0.0.1:$TEST_NGINX_REDIS_AUTH_ENABLED_PORT (default is 6380).
• perl library Redis to be installed and available in the system.

[agentzh]

@onnimonni I don't think relying on Perl's CPAN module Redis is a good idea since that CPAN module has many more dependencies than just ngx_redis2, for example. Also, I don't quite trust those Perl CPAN modules.

[agentzh]

@onnimonni After some more thinking, I think for OpenResty, we'd better completely retire the ngx_redis2, ngx_redis, and ngx_srcache modules some time in the near future. Those represent the old way of hacking things around nginx, which has no real future IMHO. And the official OpenResty team should not waste more of its time on these modules any more. We should spend energy on the future. IMHO, the ngx_lua module and the lua-resty-* libraries represent the future.

[onnimonni]

Well I believe you know the best.

Even though ngx_srcache is old it has served me really well in many projects. I think the lua-resty-* libraries are cool but for many developers they represent another kind of layer and many are not that comfortable with lua.

Also with lua I feel that the project structure gets easily really messy, the old way of hacking nginx only provides few configuration directives and those don't get that messy at least so easily.

( This might just be a local problem I have noticed in the projects where I have been working YMMV )

[agentzh]

@onnimonni I hold a similar perspective in the year 2009 ~ 2011 and wrote MANY nginx C modules like those, but later I realized the nginx C module route has no future and the Lua route can give not only much more flexibility but also even better performance in general.

[agentzh]

@onnimonni For example, the ngx_srcache module and redis can never really handle large resources like video streams efficiently so such things can never scale to CDN level caching requirements. I love truly scalable things that can handle things from tiny and highly fragmented resources to extremely large resources like video streams.

[agentzh]

@onnimonni The current implementations of lua-resty-redis and cosockets are not very efficient. But these things can easily get enormously faster. I just didn't get the time to do this. We'll get to that pretty soon.

[agentzh]

@onnimonni At that point, we'll even abandon the standard ngx_proxy module for many industrial proxying requirements.

[onnimonni]

This sounds really cool!

The thing is that I'm not working for internet scale CDN provider. I would want to get semiscalable cache which can handle ~100-500K requests per minute and which implement something like X-Cache-Tags and I'm going to do this probably with redis + nginx.

What would you suggest for this?

[agentzh]

@onnimonni I was talking about scalability. Such techs can surely be used for little trivial stuff at the same time. 500K req/min is not something impressive at all on modern hardware.