Skip to content

Commit 1d99787

Browse files
yatsukhnenkoyatsukhnenko
committed
Change connect/reconnect logic
Persistant connections can be closed via close method. Connection marked as failed only after reconnection attempts.
1 parent 399edf4 commit 1d99787

File tree

8 files changed

+72
-93
lines changed

8 files changed

+72
-93
lines changed

‎cluster_library.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,7 @@ PHP_REDIS_API void
835835
cluster_free(redisCluster *c, int free_ctx TSRMLS_DC)
836836
{
837837
/* Disconnect from each node we're connected to */
838-
cluster_disconnect(c TSRMLS_CC);
838+
cluster_disconnect(c, 0 TSRMLS_CC);
839839

840840
/* Free any allocated prefix */
841841
if (c->flags->prefix) zend_string_release(c->flags->prefix);
@@ -952,7 +952,7 @@ PHP_REDIS_API int cluster_map_keyspace(redisCluster *c TSRMLS_DC) {
952952
memset(c->master, 0, sizeof(redisClusterNode*)*REDIS_CLUSTER_SLOTS);
953953
}
954954
}
955-
redis_sock_disconnect(seed TSRMLS_CC);
955+
redis_sock_disconnect(seed, 0 TSRMLS_CC);
956956
if (mapped) break;
957957
} ZEND_HASH_FOREACH_END();
958958

@@ -1070,12 +1070,12 @@ static int cluster_check_response(redisCluster *c, REDIS_REPLY_TYPE *reply_type
10701070
}
10711071

10721072
/* Disconnect from each node we're connected to */
1073-
PHP_REDIS_API void cluster_disconnect(redisCluster *c TSRMLS_DC) {
1073+
PHP_REDIS_API void cluster_disconnect(redisCluster *c, int force TSRMLS_DC) {
10741074
redisClusterNode *node;
10751075

10761076
ZEND_HASH_FOREACH_PTR(c->nodes, node) {
10771077
if (node == NULL) continue;
1078-
redis_sock_disconnect(node->sock TSRMLS_CC);
1078+
redis_sock_disconnect(node->sock, force TSRMLS_CC);
10791079
node->sock->lazy_connect = 1;
10801080
} ZEND_HASH_FOREACH_END();
10811081
}
@@ -1302,7 +1302,7 @@ PHP_REDIS_API int cluster_abort_exec(redisCluster *c TSRMLS_DC) {
13021302
while (fi) {
13031303
if (SLOT_SOCK(c,fi->slot)->mode == MULTI) {
13041304
if (cluster_send_discard(c, fi->slot TSRMLS_CC) < 0) {
1305-
cluster_disconnect(c TSRMLS_CC);
1305+
cluster_disconnect(c, 0 TSRMLS_CC);
13061306
return -1;
13071307
}
13081308
SLOT_SOCK(c,fi->slot)->mode = ATOMIC;

‎cluster_library.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ long long mstime(void);
354354
PHP_REDIS_API short cluster_send_command(redisCluster *c, short slot, const char *cmd,
355355
int cmd_len TSRMLS_DC);
356356

357-
PHP_REDIS_API void cluster_disconnect(redisCluster *c TSRMLS_DC);
357+
PHP_REDIS_API void cluster_disconnect(redisCluster *c, int force TSRMLS_DC);
358358

359359
PHP_REDIS_API int cluster_send_exec(redisCluster *c, short slot TSRMLS_DC);
360360
PHP_REDIS_API int cluster_send_discard(redisCluster *c, short slot TSRMLS_DC);

‎common.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -614,13 +614,6 @@ typedef enum _PUBSUB_TYPE {
614614
REDIS_PROCESS_RESPONSE_CLOSURE(resp_func, ctx) \
615615
}
616616

617-
#define REDIS_STREAM_CLOSE_MARK_FAILED(redis_sock) \
618-
redis_stream_close(redis_sock TSRMLS_CC); \
619-
redis_sock->stream = NULL; \
620-
redis_sock->mode = ATOMIC; \
621-
redis_sock->status = REDIS_SOCK_STATUS_FAILED; \
622-
redis_sock->watching = 0
623-
624617
/* Extended SET argument detection */
625618
#define IS_EX_ARG(a) \
626619
((a[0]=='e' || a[0]=='E') && (a[1]=='x' || a[1]=='X') && a[2]=='\0')

‎library.c

Lines changed: 59 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -133,20 +133,16 @@ redis_error_throw(RedisSock *redis_sock TSRMLS_DC)
133133
}
134134
}
135135

136-
PHP_REDIS_API void redis_stream_close(RedisSock *redis_sock TSRMLS_DC) {
137-
if (!redis_sock->persistent) {
138-
php_stream_close(redis_sock->stream);
139-
} else {
140-
php_stream_pclose(redis_sock->stream);
141-
}
142-
}
143-
144136
PHP_REDIS_API int
145137
redis_check_eof(RedisSock *redis_sock, int no_throw TSRMLS_DC)
146138
{
147139
int count;
140+
char *errmsg;
148141

149-
if (!redis_sock->stream) {
142+
if (!redis_sock || !redis_sock->stream || redis_sock->status == REDIS_SOCK_STATUS_FAILED) {
143+
if (!no_throw) {
144+
zend_throw_exception(redis_exception_ce, "Connection closed", 0 TSRMLS_CC);
145+
}
150146
return -1;
151147
}
152148

@@ -169,51 +165,47 @@ redis_check_eof(RedisSock *redis_sock, int no_throw TSRMLS_DC)
169165
/* Success */
170166
return 0;
171167
} else if (redis_sock->mode == MULTI || redis_sock->watching) {
172-
REDIS_STREAM_CLOSE_MARK_FAILED(redis_sock);
173-
if (!no_throw) {
174-
zend_throw_exception(redis_exception_ce,
175-
"Connection lost and socket is in MULTI/watching mode",
176-
0 TSRMLS_CC);
177-
}
178-
return -1;
179-
}
180-
/* TODO: configurable max retry count */
181-
for (count = 0; count < 10; ++count) {
182-
/* close existing stream before reconnecting */
183-
if (redis_sock->stream) {
184-
redis_stream_close(redis_sock TSRMLS_CC);
185-
redis_sock->stream = NULL;
186-
}
187-
// Wait for a while before trying to reconnect
188-
if (redis_sock->retry_interval) {
189-
// Random factor to avoid having several (or many) concurrent connections trying to reconnect at the same time
190-
long retry_interval = (count ? redis_sock->retry_interval : (php_rand(TSRMLS_C) % redis_sock->retry_interval));
191-
usleep(retry_interval);
192-
}
193-
/* reconnect */
194-
if (redis_sock_connect(redis_sock TSRMLS_CC) == 0) {
195-
/* check for EOF again. */
196-
errno = 0;
197-
if (php_stream_eof(redis_sock->stream) == 0) {
198-
/* If we're using a password, attempt a reauthorization */
199-
if (redis_sock->auth && resend_auth(redis_sock TSRMLS_CC) != 0) {
200-
break;
201-
}
202-
/* If we're using a non-zero db, reselect it */
203-
if (redis_sock->dbNumber && reselect_db(redis_sock TSRMLS_CC) != 0) {
204-
break;
168+
errmsg = "Connection lost and socket is in MULTI/watching mode";
169+
} else {
170+
errmsg = "Connection lost";
171+
/* TODO: configurable max retry count */
172+
for (count = 0; count < 10; ++count) {
173+
/* close existing stream before reconnecting */
174+
if (redis_sock->stream) {
175+
redis_sock_disconnect(redis_sock, 1 TSRMLS_CC);
176+
}
177+
// Wait for a while before trying to reconnect
178+
if (redis_sock->retry_interval) {
179+
// Random factor to avoid having several (or many) concurrent connections trying to reconnect at the same time
180+
long retry_interval = (count ? redis_sock->retry_interval : (php_rand(TSRMLS_C) % redis_sock->retry_interval));
181+
usleep(retry_interval);
182+
}
183+
/* reconnect */
184+
if (redis_sock_connect(redis_sock TSRMLS_CC) == 0) {
185+
/* check for EOF again. */
186+
errno = 0;
187+
if (php_stream_eof(redis_sock->stream) == 0) {
188+
/* If we're using a password, attempt a reauthorization */
189+
if (redis_sock->auth && resend_auth(redis_sock TSRMLS_CC) != 0) {
190+
errmsg = "AUTH failed while reconnecting";
191+
break;
192+
}
193+
/* If we're using a non-zero db, reselect it */
194+
if (redis_sock->dbNumber && reselect_db(redis_sock TSRMLS_CC) != 0) {
195+
errmsg = "SELECT failed while reconnecting";
196+
break;
197+
}
198+
/* Success */
199+
return 0;
205200
}
206-
/* Success */
207-
return 0;
208201
}
209202
}
210203
}
211-
/* close stream if still here */
212-
if (redis_sock->stream) {
213-
REDIS_STREAM_CLOSE_MARK_FAILED(redis_sock);
214-
}
204+
/* close stream and mark socket as failed */
205+
redis_sock_disconnect(redis_sock, 1 TSRMLS_CC);
206+
redis_sock->status = REDIS_SOCK_STATUS_FAILED;
215207
if (!no_throw) {
216-
zend_throw_exception(redis_exception_ce, "Connection lost", 0 TSRMLS_CC);
208+
zend_throw_exception(redis_exception_ce, errmsg, 0 TSRMLS_CC);
217209
}
218210
return -1;
219211
}
@@ -1427,7 +1419,7 @@ PHP_REDIS_API int redis_sock_connect(RedisSock *redis_sock TSRMLS_DC)
14271419
#endif
14281420

14291421
if (redis_sock->stream != NULL) {
1430-
redis_sock_disconnect(redis_sock TSRMLS_CC);
1422+
redis_sock_disconnect(redis_sock, 0 TSRMLS_CC);
14311423
}
14321424

14331425
tv.tv_sec = (time_t)redis_sock->timeout;
@@ -1532,28 +1524,26 @@ redis_sock_server_open(RedisSock *redis_sock TSRMLS_DC)
15321524
/**
15331525
* redis_sock_disconnect
15341526
*/
1535-
PHP_REDIS_API int redis_sock_disconnect(RedisSock *redis_sock TSRMLS_DC)
1527+
PHP_REDIS_API int
1528+
redis_sock_disconnect(RedisSock *redis_sock, int force TSRMLS_DC)
15361529
{
15371530
if (redis_sock == NULL) {
1538-
return 1;
1539-
}
1540-
1541-
redis_sock->dbNumber = 0;
1542-
if (redis_sock->stream != NULL) {
1543-
redis_sock->status = REDIS_SOCK_STATUS_DISCONNECTED;
1544-
redis_sock->watching = 0;
1545-
1546-
/* Stil valid? */
1547-
if (!redis_sock->persistent) {
1531+
return FAILURE;
1532+
} else if (redis_sock->stream) {
1533+
if (redis_sock->persistent) {
1534+
if (force) {
1535+
php_stream_pclose(redis_sock->stream);
1536+
}
1537+
} else {
15481538
php_stream_close(redis_sock->stream);
15491539
}
1550-
15511540
redis_sock->stream = NULL;
1552-
1553-
return 1;
15541541
}
1542+
redis_sock->mode = ATOMIC;
1543+
redis_sock->status = REDIS_SOCK_STATUS_DISCONNECTED;
1544+
redis_sock->watching = 0;
15551545

1556-
return 0;
1546+
return SUCCESS;
15571547
}
15581548

15591549
/**
@@ -1764,11 +1754,8 @@ PHP_REDIS_API int redis_mbulk_reply_assoc(INTERNAL_FUNCTION_PARAMETERS, RedisSoc
17641754
PHP_REDIS_API int
17651755
redis_sock_write(RedisSock *redis_sock, char *cmd, size_t sz TSRMLS_DC)
17661756
{
1767-
if (!redis_sock || redis_sock->status == REDIS_SOCK_STATUS_DISCONNECTED) {
1768-
zend_throw_exception(redis_exception_ce, "Connection closed",
1769-
0 TSRMLS_CC);
1770-
} else if (redis_check_eof(redis_sock, 0 TSRMLS_CC) == 0 &&
1771-
php_stream_write(redis_sock->stream, cmd, sz) == sz
1757+
if (redis_check_eof(redis_sock, 0 TSRMLS_CC) == 0 &&
1758+
php_stream_write(redis_sock->stream, cmd, sz) == sz
17721759
) {
17731760
return sz;
17741761
}
@@ -2044,8 +2031,8 @@ redis_sock_gets(RedisSock *redis_sock, char *buf, int buf_size,
20442031
if(php_stream_get_line(redis_sock->stream, buf, buf_size, line_size)
20452032
== NULL)
20462033
{
2047-
// Close, put our socket state into error
2048-
REDIS_STREAM_CLOSE_MARK_FAILED(redis_sock);
2034+
// Close our socket
2035+
redis_sock_disconnect(redis_sock, 1 TSRMLS_CC);
20492036

20502037
// Throw a read error exception
20512038
zend_throw_exception(redis_exception_ce, "read error on connection",

‎library.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ PHP_REDIS_API void redis_type_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *
4141
PHP_REDIS_API RedisSock* redis_sock_create(char *host, int host_len, unsigned short port, double timeout, double read_timeout, int persistent, char *persistent_id, long retry_interval, zend_bool lazy_connect);
4242
PHP_REDIS_API int redis_sock_connect(RedisSock *redis_sock TSRMLS_DC);
4343
PHP_REDIS_API int redis_sock_server_open(RedisSock *redis_sock TSRMLS_DC);
44-
PHP_REDIS_API int redis_sock_disconnect(RedisSock *redis_sock TSRMLS_DC);
44+
PHP_REDIS_API int redis_sock_disconnect(RedisSock *redis_sock, int force TSRMLS_DC);
4545
PHP_REDIS_API zval *redis_sock_read_multibulk_reply_zval(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab);
4646
PHP_REDIS_API char *redis_sock_read_bulk_reply(RedisSock *redis_sock, int bytes TSRMLS_DC);
4747
PHP_REDIS_API int redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *_z_tab, void *ctx);
@@ -62,7 +62,6 @@ PHP_REDIS_API int redis_unsubscribe_response(INTERNAL_FUNCTION_PARAMETERS,
6262
RedisSock *redis_sock, zval *z_tab, void *ctx);
6363

6464
PHP_REDIS_API int redis_sock_write(RedisSock *redis_sock, char *cmd, size_t sz TSRMLS_DC);
65-
PHP_REDIS_API void redis_stream_close(RedisSock *redis_sock TSRMLS_DC);
6665
PHP_REDIS_API int redis_check_eof(RedisSock *redis_sock, int no_throw TSRMLS_DC);
6766
PHP_REDIS_API RedisSock *redis_sock_get(zval *id TSRMLS_DC, int nothrow);
6867
PHP_REDIS_API void redis_free_socket(RedisSock *redis_sock);

‎redis.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ free_redis_object(void *object TSRMLS_DC)
536536

537537
zend_object_std_dtor(&redis->std TSRMLS_CC);
538538
if (redis->sock) {
539-
redis_sock_disconnect(redis->sock TSRMLS_CC);
539+
redis_sock_disconnect(redis->sock, 0 TSRMLS_CC);
540540
redis_free_socket(redis->sock);
541541
}
542542
efree(redis);
@@ -577,7 +577,7 @@ free_redis_object(zend_object *object)
577577

578578
zend_object_std_dtor(&redis->std TSRMLS_CC);
579579
if (redis->sock) {
580-
redis_sock_disconnect(redis->sock TSRMLS_CC);
580+
redis_sock_disconnect(redis->sock, 0 TSRMLS_CC);
581581
redis_free_socket(redis->sock);
582582
}
583583
}
@@ -946,7 +946,7 @@ redis_connect(INTERNAL_FUNCTION_PARAMETERS, int persistent)
946946
redis = PHPREDIS_GET_OBJECT(redis_object, object);
947947
/* if there is a redis sock already we have to remove it */
948948
if (redis->sock) {
949-
redis_sock_disconnect(redis->sock TSRMLS_CC);
949+
redis_sock_disconnect(redis->sock, 0 TSRMLS_CC);
950950
redis_free_socket(redis->sock);
951951
}
952952

@@ -993,7 +993,7 @@ PHP_METHOD(Redis, close)
993993
{
994994
RedisSock *redis_sock = redis_sock_get_connected(INTERNAL_FUNCTION_PARAM_PASSTHRU);
995995

996-
if (redis_sock && redis_sock_disconnect(redis_sock TSRMLS_CC)) {
996+
if (redis_sock && redis_sock_disconnect(redis_sock, 1 TSRMLS_CC)) {
997997
RETURN_TRUE;
998998
}
999999
RETURN_FALSE;

‎redis_cluster.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ PHP_METHOD(RedisCluster, __construct) {
542542

543543
/* {{{ proto bool RedisCluster::close() */
544544
PHP_METHOD(RedisCluster, close) {
545-
cluster_disconnect(GET_CONTEXT() TSRMLS_CC);
545+
cluster_disconnect(GET_CONTEXT(), 1 TSRMLS_CC);
546546
RETURN_TRUE;
547547
}
548548

‎redis_session.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ redis_pool_free(redis_pool *pool TSRMLS_DC) {
126126
rpm = pool->head;
127127
while (rpm) {
128128
next = rpm->next;
129-
redis_sock_disconnect(rpm->redis_sock TSRMLS_CC);
129+
redis_sock_disconnect(rpm->redis_sock, 0 TSRMLS_CC);
130130
redis_free_socket(rpm->redis_sock);
131131
if (rpm->prefix) zend_string_release(rpm->prefix);
132132
if (rpm->auth) zend_string_release(rpm->auth);

0 commit comments

Comments
 (0)