cmcfarlen commented on code in PR #11815:
URL: https://github.com/apache/trafficserver/pull/11815#discussion_r1801682940
##########
src/shared/rpc/IPCSocketClient.cc:
##########
@@ -117,23 +117,36 @@ 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 {};
Review Comment:
I know it's not part of the changes, but this will return `NO_ERROR`? But
the comment indicates a failure. Could `UNKNOWN` be a better return?
##########
src/shared/rpc/IPCSocketClient.cc:
##########
@@ -117,23 +117,36 @@ 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 {};
}
MessageStorage<356000> bs;
-
- ReadStatus readStatus{ReadStatus::UNKNOWN};
+ int attempts_left{attempts};
+ ReadStatus readStatus{ReadStatus::NO_ERROR};
while (true) {
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));
+ if (ret < 0 && (errno == EAGAIN || errno == EINTR)) {
+ if (auto r = read_ready(_sock, timeout_ms.count()); r <= 0) {
+ readStatus = r == 0 ? ReadStatus::TIMEOUT : ReadStatus::READ_ERROR;
+ --attempts_left;
+ }
+ } else {
+ // done!
+ break;
+ }
+ } while (ret != 0 && attempts_left > 0);
Review Comment:
Should this check for readStatus timeout and error and also stop the loop?
If you don't check for timeout, this function will wait for timeout_ms *
attempts (10 seconds by default). What errors would poll return that we would
want to retry?
##########
src/shared/rpc/IPCSocketClient.cc:
##########
@@ -30,12 +30,12 @@
#include "shared/rpc/MessageStorage.h"
#include <tscore/ink_assert.h>
#include <tscore/ink_sock.h>
-
+#include <iostream>
Review Comment:
This header is unused.
--
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
