brbzull0 commented on code in PR #11815:
URL: https://github.com/apache/trafficserver/pull/11815#discussion_r1825726296
##########
src/shared/rpc/IPCSocketClient.cc:
##########
@@ -117,35 +117,52 @@ IPCSocketClient ::send(std::string_view data)
}
IPCSocketClient::ReadStatus
-IPCSocketClient::read_all(std::string &content)
+IPCSocketClient::read_all(std::string &content, std::chrono::milliseconds
timeout_ms, int attempts)
{
if (this->is_closed()) {
// we had a failure.
- return {};
+ return ReadStatus::UNKNOWN;
}
MessageStorage<356000> bs;
-
- ReadStatus readStatus{ReadStatus::UNKNOWN};
- while (true) {
+ int attempts_left{attempts};
+ ReadStatus readStatus{ReadStatus::NO_ERROR};
+ // Try to read all the data from the socket. If a timeout happens we retry
+ // 'attemps' times. On error we just stop.
+ while (attempts_left > 0 || readStatus == ReadStatus::NO_ERROR) {
auto buf = bs.writable_data();
const auto to_read = bs.available(); // Available in the current memory
chunk.
- ssize_t ret{-1};
- do {
- ret = ::read(_sock, buf, to_read);
- } while (ret < 0 && (errno == EAGAIN || errno == EINTR));
+ ssize_t nread{-1};
- if (ret > 0) {
- bs.save(ret);
+ // Try again if timed out.
+ if (auto const r = read_ready(_sock, timeout_ms.count()); r == 0) {
+ readStatus = ReadStatus::TIMEOUT;
+ --attempts_left;
continue;
- } else {
- if (bs.stored() > 0) {
- readStatus = ReadStatus::NO_ERROR;
- break;
+ } else if (r < 0) {
+ // No more tries.
+ readStatus = ReadStatus::READ_ERROR;
+ break;
+ }
+
+ nread = ::read(_sock, buf, to_read);
+ if (nread > 0) {
+ bs.save(nread);
+ continue;
Review Comment:
if you have a message for instance, to fetch records, or metrics and you
have more than 6k metrics, then `10s` seems rasonable as it could take a while
to send all the strings over the pipe.
I would expect `read` return `EOF` once done, if it's done then `read_all`
knows there is no more data to read(ofc). If there is any error different than
EAGAIN then this will just stop and fail.
if you think tehre is something that I am missing, please let me know. I'll
be happy to address.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
