Skip to content

Added lazy_connect option to RedisArray#303

Merged
michael-grunder merged 17 commits into
phpredis:developfrom
mobli:develop
Aug 31, 2013
Merged

Added lazy_connect option to RedisArray#303
michael-grunder merged 17 commits into
phpredis:developfrom
mobli:develop

Conversation

@mobli
Copy link
Copy Markdown
Contributor

@mobli mobli commented Feb 24, 2013

Hi Michael,

So here we go, my first try after using git flow.

Here is the summary of the changes I'm proposing:
Added an option to let each RedisArray connection connect lazily to their respective server. This is useful then working with a redis cluster composed of many shards which are not necessarily in use all at once.

Cheers!

Emmanuel

Emmanuel Merali added 9 commits January 21, 2013 18:10
Added the possibility to delay each reconnection attempt, including a
random factor to prevent several or many concurrent connections from
trying to reconnect at the same time.
Added the select command to RedisArray to select a DB on every
connections in one instruction.
Added the possibility to delay each reconnection attempt, including a
random factor to prevent several or many concurrent connections from
trying to reconnect at the same time.
Added the select command to RedisArray to select a DB on every
connections in one instruction.
Also, fixed a compiler warning:
redis_array_impl.c:1115:15: warning: incompatible pointer types
assigning to 'zval **' (aka 'struct _zval_struct **') from 'zval
**(*)[2]' [-Wincompatible-pointer-types]
Added the possibility to delay each reconnection attempt, including a
random factor to prevent several or many concurrent connections from
trying to reconnect at the same time.
Added the select command to RedisArray to select a DB on every
connections in one instruction.
Also, fixed a compiler warning:
redis_array_impl.c:1115:15: warning: incompatible pointer types
assigning to 'zval **' (aka 'struct _zval_struct **') from 'zval
**(*)[2]' [-Wincompatible-pointer-types]
Added an option to let each RedisArray connection connect lazily to
their respective server. This is useful then working with a redis
cluster composed of many shards which are not necessarily in use all at
once.
Conflicts:
	library.c
	library.h
	redis.c
	redis_array.c
	redis_array_impl.c
	redis_array_impl.h
	redis_session.c
Added an option to let each RedisArray connection connect lazily to
their respective server. This is useful then working with a redis
cluster composed of many shards which are not necessarily in use all at
once.
@mobli
Copy link
Copy Markdown
Contributor Author

mobli commented Feb 24, 2013

Also, the extension doesn't compile in the develop branch. I've compiled locally by making some changes to library.c, but I didn't want to commit them because I'm not sure what the intention is. For instance:

library.c:707:21: error: void function 'redis_client_list_reply' should not return a value

I replaced

return -1

with

RETURN_FALSE

Also in

library.c:852:13: error: non-void function 'redis_sock_read_multibulk_reply_zipped_with_flag' should return a value

I simply removed the lines

        } else {
            RETURN_FALSE;

Same for the remaining errors.
So, it compiles but I'm not sure what it breaks...

@michael-grunder
Copy link
Copy Markdown
Member

Nice spot! Those are things that I broke :) I'll get them sorted for sure.

I think we're using Redis in a similar way (tons of shards, all over the place). I thing an option to connect lazily is a good thing too.

Cheers!
Mike

@mobli
Copy link
Copy Markdown
Contributor Author

mobli commented Feb 25, 2013

Hi Michael (can I call you Mike? :-))
My concern is that the code that breaks compilation is also in master. If I want to build a clean "official" redis.so, I can't just clone the repo now. What do I need to do?

Thanks,

Emmanuel

@michael-grunder
Copy link
Copy Markdown
Member

Hey there,

Yes, Michael, Mike, or "hey you" are all acceptable. :)

I have merged the client reply fix into develop and hotfixed the zipped_with_reply issue.

Cheers dude,
Mike

@mobli
Copy link
Copy Markdown
Contributor Author

mobli commented Feb 25, 2013

Hi again,

There are still 3 compilation errors of the same kind (non-void function 'bla-bla' should return a value) on lines 1096, 1143 and 1222. I don't really get the point of these, why not simply returning -1?

Cheers,

Emmanuel

michael-grunder added a commit that referenced this pull request Feb 25, 2013
methods are int returns

Resolves issues pertaining to #303
@michael-grunder
Copy link
Copy Markdown
Member

Hey dude, the reason that it's not simply returning -1 is that we're setting the return value for the PHP userland caller. If we're not in a pipeline we want PHP to return FALSE. if we are in a pipeline, we want to add FALSE to the array of pipeline/multi replies.

This looks to get all of them.

Cheers,
Mike

@mobli
Copy link
Copy Markdown
Contributor Author

mobli commented Feb 25, 2013

Aha! I get it! Thanks for the explanation, now it all makes sense!

Have a good night now (if you ever sleep) and thanks a lot for the really fast turnaround!

Take care,

Emmanuel

Emmanuel Merali added 8 commits February 25, 2013 04:06
An integrated distributor for easy resharding.
Works on the principle of redistributing keys from 1 shard to 2 shards
evenly.
Updated documentation
Updated documentation
Updated documentation
FIx C99 compliance
Changed distributor to accept strings as well as longs so that
definitions parsed from parse_ini_files may be used as is
Fix poor performance on initial use of easy reshard distributor
mechanism.
@michael-grunder
Copy link
Copy Markdown
Member

Hey there,

Sorry for such a long delay. I have merged and integrated your changes into the feature branch feature/mobli_ra_changes along with fixing some conflicts. I would like to do some testing before getting this into develop and then into master, but I figured I would let you know.

Cheers!
Mike

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