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

Coroutine\Http\Client isConnected() 建議 #2184

Closed
s890510 opened this issue Dec 5, 2018 · 11 comments
Closed

Coroutine\Http\Client isConnected() 建議 #2184

s890510 opened this issue Dec 5, 2018 · 11 comments

Comments

@s890510
Copy link

s890510 commented Dec 5, 2018

最近在調整ConnectionPool,有發現到如果Server端終止連線後,Client端會不清楚
因此能否在 isConnected 加上

http_client_coro_property *hcc = (http_client_coro_property *) swoole_get_property(getThis(), 0);

char data;
if(hcc->socket->recv(&data, 1) == 0) {
	RETURN_FALSE;
}

來做輔助判斷呢?

@s890510 s890510 changed the title Coroutine\Http\Client isConnected() Coroutine\Http\Client isConnected() 建議 Dec 5, 2018
@twose
Copy link
Member

twose commented Dec 6, 2018

不可取, 为什么不用PHP代码实现? 这是异步时代的残留物, 协程和同步客户端是表现一致的.

@s890510 s890510 closed this as completed Dec 6, 2018
@s890510 s890510 reopened this Dec 6, 2018
@s890510 s890510 closed this as completed Dec 6, 2018
@s890510
Copy link
Author

s890510 commented Dec 6, 2018

@twose
之所以不用PHP代碼實現是因為在整個 coroutine\http\client 無法直接採用recv來進行這動作,
coroutine\http\client 的recv() 根據文件是必須在setDefer或是webSocket模式下才能使用,
這部分無論使用上或是回傳內容都與Socket下的 recv有些許差別!

因為isConnected的使用時機大都是從Pool撈起來,在使用前進行狀態確認使用,
雖然可在得知對口Server單方面連接關閉後,將pool中內容清除重置來解決下一個撈出來的client亦是無法使用的問題
但這問題若能提早去取得狀態資訊不是更好嗎?

當然,如果認為這可用來輔助Check的殘留物是不好的,那我也沒意見
(不過或許你們有什麼解法可以去針對這部分做解決?)

@s890510 s890510 reopened this Dec 6, 2018
@twose
Copy link
Member

twose commented Dec 6, 2018

我忘了提http client是自动连接的(误以为是client), 开发者不需要感知是否连接, 这个没有意义, 就像使用curl的时候你不需要去关心连接.

@twose twose added the question label Dec 6, 2018
@twose
Copy link
Member

twose commented Dec 6, 2018

你是指连接断开之后的下一次请求会失败吗?

@s890510
Copy link
Author

s890510 commented Dec 6, 2018

是的,此時所回傳的errorCode為 -3 (這部分isConnected是無法偵測到)
(Nginx default為75s,不同Server的設定值也會有所不同)

(When keep-alive is true)

@twose
Copy link
Member

twose commented Dec 6, 2018

@s890510 协程改造以后其表现和同步完全一致, 所以没有监测到连接断开事件, 这里需要额外处理, 底层应该自动进行重连, 连接不是客户端需要关心的事.

感谢详细的反馈, 我差点错过了这个问题

@s890510
Copy link
Author

s890510 commented Dec 6, 2018

提供可簡單複現的code

go(function() {
	$pool = Swlib\Saber::create([
        'base_uri' => 'https://pecl.php.net',
        'use_pool' => true,
		'keep_alive' => true
    ]);
	
	$result = $pool->get('/');
	echo $result->statusCode."\n";
	
	Swoole\Coroutine::sleep(75);
	
	$result = $pool->get('/');
	echo $result->statusCode."\n";
});

Response:

200
PHP Fatal error:  Uncaught Swlib\Http\Exception\ConnectException: Connection is forcibly cut off by the remote server in /home/evan/swoole_docker/src/vendor/swlib/saber/src/Request.php:641
Stack trace:

@twose
Copy link
Member

twose commented Dec 6, 2018

是的 在长连接处理上出现了向下不兼容情况

@s890510
Copy link
Author

s890510 commented Dec 20, 2018

最近又發現...
Swoole\Coroutine\Redis()
也是會有類似問題

雖然已經先透過ping()做一次確認了,
但是還是有機會跑出 Code: 3, Msg: Server closed the connection

@twose
Copy link
Member

twose commented Dec 24, 2018

@s890510 更新到4.2.10, http 和 redis 底层都增加了keep_liveness, 会在请求前主动侦测连接是否被close.

注: isConnect作为一个没有在文档中声明且不符合设计的调试API已被删除

@s890510
Copy link
Author

s890510 commented Dec 24, 2018

目前測試看起來沒有問題! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants