Skip to content

Commit

Permalink
Small cleanup for error message. (dmlc#10502)
Browse files Browse the repository at this point in the history
- The `Fail` function can handle file location automatically.
- Report concatenated error for connection poll.
- Typos.
  • Loading branch information
trivialfis authored Jul 2, 2024
1 parent 804cf85 commit 5f0c1e9
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/c_api/coll_c_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ XGB_DLL int XGTrackerFree(TrackerHandle handle) {
//
// - We don't have the first case since we never access the raw pointer.
//
// - We don't hve the second case for most of the scenarios since tracker is an unique
// - We don't have the second case for most of the scenarios since tracker is an unique
// object, if the free function is called before another function calls, it's likely
// to be a bug in the user code. The use_count should only decrease in this function.
while (ptr->first.use_count() != 1) {
Expand Down
29 changes: 10 additions & 19 deletions src/collective/socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@
#include "xgboost/collective/socket.h"

#include <array> // for array
#include <cstddef> // std::size_t
#include <cstdint> // std::int32_t
#include <cstring> // std::memcpy, std::memset
#include <filesystem> // for path
#include <cstddef> // for size_t
#include <cstdint> // for int32_t
#include <cstring> // for memcpy, memset
#include <system_error> // for error_code, system_category
#include <thread> // for sleep_for

Expand All @@ -26,8 +25,7 @@ SockAddress MakeSockAddress(StringView host, in_port_t port) {
struct addrinfo *res = nullptr;
int sig = getaddrinfo(host.c_str(), nullptr, &hints, &res);
if (sig != 0) {
LOG(FATAL) << "Failed to get addr info for: " << host
<< ", error: " << gai_strerror(sig);
LOG(FATAL) << "Failed to get addr info for: " << host << ", error: " << gai_strerror(sig);
return {};
}
if (res->ai_family == static_cast<std::int32_t>(SockDomain::kV4)) {
Expand Down Expand Up @@ -129,10 +127,9 @@ std::size_t TCPSocket::Send(StringView str) {
}

Result last_error;
auto log_failure = [&host, &last_error, port](Result err, char const *file, std::int32_t line) {
auto log_failure = [&host, &last_error, port](Result err) {
last_error = std::move(err);
LOG(WARNING) << std::filesystem::path{file}.filename().string() << "(" << line
<< "): Failed to connect to:" << host << ":" << port
LOG(WARNING) << "Failed to connect to:" << host << ":" << port
<< " Error:" << last_error.Report();
};

Expand All @@ -149,8 +146,7 @@ std::size_t TCPSocket::Send(StringView str) {

auto errcode = system::LastError();
if (!system::ErrorWouldBlock(errcode)) {
log_failure(Fail("connect failed.", std::error_code{errcode, std::system_category()}),
__FILE__, __LINE__);
log_failure(Fail("connect failed.", std::error_code{errcode, std::system_category()}));
continue;
}

Expand All @@ -160,21 +156,16 @@ std::size_t TCPSocket::Send(StringView str) {
if (!result.OK()) {
// poll would fail if there's a socket error, we log the root cause instead of the
// poll failure.
auto sockerr = conn.GetSockError();
if (!sockerr.OK()) {
result = std::move(sockerr);
}
log_failure(std::move(result), __FILE__, __LINE__);
log_failure(std::move(result) + conn.GetSockError());
continue;
}
if (!poll.CheckWrite(conn)) {
log_failure(Fail("poll failed.", std::error_code{errcode, std::system_category()}), __FILE__,
__LINE__);
log_failure(Fail("poll failed.", std::error_code{errcode, std::system_category()}));
continue;
}
result = conn.GetSockError();
if (!result.OK()) {
log_failure(std::move(result), __FILE__, __LINE__);
log_failure(std::move(result));
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion src/collective/tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ Result RabitTracker::Bootstrap(std::vector<WorkerProxy>* p_workers) {
addresses.emplace_back(SockAddrV6{*ipv6});
}
}
// If not v4 address is found, we try v6
// If no v4 address is found, we try v6
for (auto const& addr : addresses) {
if (addr.IsV6()) {
auto ip = addr.V6().Addr();
Expand Down

0 comments on commit 5f0c1e9

Please sign in to comment.