Support progressive read timeout bthread handler #3163
Conversation
|
Hi @zchuang185 Thank you for you contribution! I have left some comments, but I think the biggest problem is that this solution is too heavy and too intrusive to Controller. Is there any other solution? For example, after rpc success, set a bthread_timer with progressive_read_timeout_ms and check the Socket when timeout? |
|
@wwbmmm I've added ProgressiveTimeoutRead which is a ProgressiveReader wrapper class, when OnReadPart is invoked, it sets up a bthread_timer to monitor the idle timeout for the progressive socket reader. ProgressiveTimeoutRead is only activated when progressive_read_timeout_ms has been explicitly configured. |
src/brpc/controller.cpp
Outdated
| << " pre_idle_duration_us : " << pre_idle_duration_us; | ||
| return; | ||
| } | ||
| reader->set_read_timeout(); |
There was a problem hiding this comment.
There is a thread safety issue here. The process will crash if the reader has already been destructed.
There was a problem hiding this comment.
This issue deserves attention.
* change the timeout checker bthread to timer bthread * refine ProgressiveReadTimeoutReader class hold SocketId and read_timeout_ms fields * refine socektId access method, change HandleIdleProgressiveReader belong and logic
| } | ||
| add_flag(FLAGS_PROGRESSIVE_READER); | ||
| if (progressive_read_timeout_ms() > 0) { | ||
| auto reader = new ProgressiveTimeoutReader(_rpa->GetSocketId(), _progressive_read_timeout_ms, r); |
There was a problem hiding this comment.
I believe that _current_call.peer_id can be used instead of GetSocketId.
* change the timeout checker bthread to timer bthread * refine ProgressiveReadTimeoutReader class hold SocketId and read_timeout_ms fields * refine socektId access method, change HandleIdleProgressiveReader belong and logic
3 participants
What problem does this PR solve?
Issue Number: resolve #3133
Problem Summary: when Controller response_will_be_read_progressively() it needs background bthread monitor handler to solve progressive reader idle timeout，when hit the progressive reader idle timeout duration should close current client socket connection.
What is changed and the side effects?
Changed:
Side effects:
Performance effects: NO
Breaking backward compatibility: NO
Check List: