Skip to content

Commit 542ca06

Browse files
committed
Better logging for delivery fenced writer and actual exception safety of PerformIO method
commit_hash:6420854eada61659e5d217fdbe73705d5715e0fa
1 parent 21733f5 commit 542ca06

File tree

1 file changed

+34
-23
lines changed

1 file changed

+34
-23
lines changed

yt/yt/core/net/connection.cpp

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ using namespace NConcurrency;
4848

4949
////////////////////////////////////////////////////////////////////////////////
5050

51+
static constexpr auto& Logger = NetLogger;
52+
53+
////////////////////////////////////////////////////////////////////////////////
54+
5155
namespace {
5256

5357
int GetLastNetworkError()
@@ -93,39 +97,40 @@ ssize_t WriteToFD(TFileDescriptor fd, const char* buffer, size_t length)
9397
#endif
9498
}
9599

96-
enum class EPipeReadStatus
97-
{
98-
PipeEmpty,
99-
PipeNotEmpty,
100-
NotSupportedError,
101-
};
102-
103-
EPipeReadStatus CheckPipeReadStatus(const TString& pipePath)
100+
TErrorOr<int> CheckPipeBytesLeftToRead(const TString& pipePath) noexcept
104101
{
105102
#ifdef _linux_
106103
int bytesLeft = 0;
107104

105+
auto makeSystemError = [&] (TFormatString<> message) {
106+
return TError(message)
107+
<< TError::FromSystem()
108+
<< TErrorAttribute("pipe_path", pipePath);
109+
};
110+
108111
{
109112
int flags = O_RDONLY | O_CLOEXEC | O_NONBLOCK;
110113
int fd = HandleEintr(::open, pipePath.c_str(), flags);
111114

115+
if (fd == -1) {
116+
return makeSystemError("Failed to open file descriptor");
117+
}
118+
112119
int ret = ::ioctl(fd, FIONREAD, &bytesLeft);
113-
if (ret == -1 && errno == EINVAL) {
114-
// Some linux platforms do not support
115-
// FIONREAD call. In such cases we
116-
// expect EINVAL error.
117-
return EPipeReadStatus::NotSupportedError;
120+
121+
if (ret == -1) {
122+
return makeSystemError("ioctl failed");
118123
}
119124

120-
SafeClose(fd, /*ignoreBadFD*/ false);
125+
if (!TryClose(fd, /*ignoreBadFD*/ false)) {
126+
return makeSystemError("Failed to close file descriptor");
127+
}
121128
}
122129

123-
return bytesLeft == 0
124-
? EPipeReadStatus::PipeEmpty
125-
: EPipeReadStatus::PipeNotEmpty;
130+
return bytesLeft;
126131
#else
127132
Y_UNUSED(pipePath);
128-
return EPipeReadStatus::NotSupportedError;
133+
return TError("Unsupported platform");
129134
#endif
130135
}
131136

@@ -340,13 +345,19 @@ class TDeliveryFencedWriteOperation
340345
{
341346
auto result = TWriteOperation::PerformIO(fd);
342347
if (IsWriteComplete(result)) {
343-
auto pipeReadStatus = CheckPipeReadStatus(PipePath_);
344-
if (pipeReadStatus == EPipeReadStatus::NotSupportedError) {
345-
return TError("Delivery fenced write failed: FIONDREAD is not supported on your platform")
346-
<< TError::FromSystem();
348+
auto bytesLeftOrError = CheckPipeBytesLeftToRead(PipePath_);
349+
350+
351+
if (!bytesLeftOrError.IsOK()) {
352+
YT_LOG_ERROR(bytesLeftOrError, "Delivery fenced write failed");
353+
return std::move(bytesLeftOrError).Wrap();
354+
} else {
355+
YT_LOG_DEBUG("Delivery fenced write pipe check finished (BytesLeft: %v)", bytesLeftOrError.Value());
347356
}
348357

349-
result.Value().Retry = (pipeReadStatus != EPipeReadStatus::PipeEmpty);
358+
result.Value().Retry = (bytesLeftOrError.Value() != 0);
359+
} else {
360+
YT_LOG_DEBUG("Delivery fenced write to pipe step finished (Result: %v)", result);
350361
}
351362

352363
return result;

0 commit comments

Comments
 (0)