This is an automated email from the ASF dual-hosted git repository.
ethanfeng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/celeborn.git
The following commit(s) were added to refs/heads/main by this push:
new 6853b23b4 [CELEBORN-1819][CIP-14] Refactor cppClient with nested
namespace
6853b23b4 is described below
commit 6853b23b49f7142ce6cc7d44ad4a41b6de769e3a
Author: HolyLow <[email protected]>
AuthorDate: Sat Jan 4 15:56:45 2025 +0800
[CELEBORN-1819][CIP-14] Refactor cppClient with nested namespace
### What changes were proposed in this pull request?
This PR refactors cppClient to use nested namespace.
### Why are the changes needed?
The nested namespace would improve symbol isolation and make the
celebornCpp project more extensible.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Compilation and UTs.
Closes #3050 from
HolyLow/issue/celeborn-1819-refactor-cppClient-with-nested-namespace.
Authored-by: HolyLow <[email protected]>
Signed-off-by: mingji <[email protected]>
---
cpp/celeborn/conf/BaseConf.cpp | 2 +
cpp/celeborn/conf/BaseConf.h | 2 +
cpp/celeborn/conf/CMakeLists.txt | 6 +-
cpp/celeborn/conf/CelebornConf.cpp | 11 +-
cpp/celeborn/conf/CelebornConf.h | 3 +
cpp/celeborn/conf/tests/BaseConfTest.cpp | 20 +--
cpp/celeborn/conf/tests/CelebornConfTest.cpp | 4 +-
cpp/celeborn/memory/ByteBuffer.cpp | 4 +-
cpp/celeborn/memory/ByteBuffer.h | 2 +
cpp/celeborn/memory/CMakeLists.txt | 1 +
cpp/celeborn/memory/tests/ByteBufferTest.cpp | 2 +-
cpp/celeborn/protocol/PartitionLocation.cpp | 2 +
cpp/celeborn/protocol/PartitionLocation.h | 2 +
cpp/celeborn/protocol/StatusCode.h | 4 +-
cpp/celeborn/protocol/TransportMessage.cpp | 11 +-
cpp/celeborn/protocol/TransportMessage.h | 4 +-
.../protocol/tests/PartitionLocationTest.cpp | 2 +-
.../protocol/tests/TransportMessageTest.cpp | 1 -
cpp/celeborn/utils/CMakeLists.txt | 10 +-
cpp/celeborn/utils/CelebornException.cpp | 2 +
cpp/celeborn/utils/CelebornException.h | 2 +
cpp/celeborn/utils/CelebornUtils.h | 3 +-
cpp/celeborn/utils/Exceptions.cpp | 12 +-
cpp/celeborn/utils/Exceptions.h | 176 +++++++++++----------
cpp/celeborn/utils/StackTrace.cpp | 6 +-
cpp/celeborn/utils/StackTrace.h | 6 +-
cpp/celeborn/utils/tests/ExceptionTest.cpp | 102 ++++++------
27 files changed, 226 insertions(+), 176 deletions(-)
diff --git a/cpp/celeborn/conf/BaseConf.cpp b/cpp/celeborn/conf/BaseConf.cpp
index 9bade6eba..9d49261cd 100644
--- a/cpp/celeborn/conf/BaseConf.cpp
+++ b/cpp/celeborn/conf/BaseConf.cpp
@@ -29,6 +29,7 @@ namespace fs = std::experimental::filesystem;
#include "celeborn/utils/CelebornUtils.h"
namespace celeborn {
+namespace conf {
namespace core {
folly::Optional<std::string> MemConfig::get(const std::string& key) const {
@@ -225,4 +226,5 @@ void BaseConf::checkRegisteredProperties(
<< str;
}
}
+} // namespace conf
} // namespace celeborn
diff --git a/cpp/celeborn/conf/BaseConf.h b/cpp/celeborn/conf/BaseConf.h
index 4ad4f8793..73c338c22 100644
--- a/cpp/celeborn/conf/BaseConf.h
+++ b/cpp/celeborn/conf/BaseConf.h
@@ -30,6 +30,7 @@
#include "celeborn/utils/Exceptions.h"
namespace celeborn {
+namespace conf {
class Config {
public:
@@ -260,4 +261,5 @@ class BaseConf {
registeredProps_;
};
+} // namespace conf
} // namespace celeborn
diff --git a/cpp/celeborn/conf/CMakeLists.txt b/cpp/celeborn/conf/CMakeLists.txt
index 2d708dd87..f9fbca886 100644
--- a/cpp/celeborn/conf/CMakeLists.txt
+++ b/cpp/celeborn/conf/CMakeLists.txt
@@ -12,7 +12,11 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
-add_library(conf BaseConf.cpp CelebornConf.cpp)
+add_library(
+ conf
+ STATIC
+ BaseConf.cpp
+ CelebornConf.cpp)
target_link_libraries(
conf
diff --git a/cpp/celeborn/conf/CelebornConf.cpp
b/cpp/celeborn/conf/CelebornConf.cpp
index 01a9f9713..385bdba40 100644
--- a/cpp/celeborn/conf/CelebornConf.cpp
+++ b/cpp/celeborn/conf/CelebornConf.cpp
@@ -20,6 +20,8 @@
#include "celeborn/conf/CelebornConf.h"
namespace celeborn {
+namespace conf {
+using Duration = utils::Duration;
namespace {
// folly::to<> does not generate 'true' and 'false', so we do it ourselves.
@@ -167,21 +169,21 @@ void CelebornConf::registerProperty(
}
Timeout CelebornConf::rpcLookupTimeout() const {
- return toTimeout(toDuration(optionalProperty(kRpcLookupTimeout).value()));
+ return
utils::toTimeout(toDuration(optionalProperty(kRpcLookupTimeout).value()));
}
Timeout CelebornConf::clientRpcGetReducerFileGroupRpcAskTimeout() const {
- return toTimeout(toDuration(
+ return utils::toTimeout(toDuration(
optionalProperty(kClientRpcGetReducerFileGroupRpcAskTimeout).value()));
}
Timeout CelebornConf::networkConnectTimeout() const {
- return toTimeout(
+ return utils::toTimeout(
toDuration(optionalProperty(kNetworkConnectTimeout).value()));
}
Timeout CelebornConf::clientFetchTimeout() const {
- return toTimeout(toDuration(optionalProperty(kClientFetchTimeout).value()));
+ return
utils::toTimeout(toDuration(optionalProperty(kClientFetchTimeout).value()));
}
int CelebornConf::networkIoNumConnectionsPerPeer() const {
@@ -195,4 +197,5 @@ int CelebornConf::networkIoClientThreads() const {
int CelebornConf::clientFetchMaxReqsInFlight() const {
return std::stoi(optionalProperty(kClientFetchMaxReqsInFlight).value());
}
+} // namespace conf
} // namespace celeborn
diff --git a/cpp/celeborn/conf/CelebornConf.h b/cpp/celeborn/conf/CelebornConf.h
index 68ce6c8aa..7f36823fa 100644
--- a/cpp/celeborn/conf/CelebornConf.h
+++ b/cpp/celeborn/conf/CelebornConf.h
@@ -21,6 +21,8 @@
#include "celeborn/utils/CelebornUtils.h"
namespace celeborn {
+using Timeout = utils::Timeout;
+namespace conf {
/***
* steps to add a new config:
* === in CelebornConf.h:
@@ -82,4 +84,5 @@ class CelebornConf : public BaseConf {
int clientFetchMaxReqsInFlight() const;
};
+} // namespace conf
} // namespace celeborn
diff --git a/cpp/celeborn/conf/tests/BaseConfTest.cpp
b/cpp/celeborn/conf/tests/BaseConfTest.cpp
index 52c75c214..0f0484f0c 100644
--- a/cpp/celeborn/conf/tests/BaseConfTest.cpp
+++ b/cpp/celeborn/conf/tests/BaseConfTest.cpp
@@ -19,7 +19,9 @@
#include "celeborn/conf/BaseConf.h"
-using namespace celeborn;
+using namespace celeborn::conf;
+
+using CelebornUserError = celeborn::utils::CelebornUserError;
// As the BaseConf's construction is protected, we expose it explicitly.
class TestedBaseConf : public BaseConf {};
@@ -30,16 +32,16 @@ void testInsertToConf(TestedBaseConf* conf, bool isMutable
= true) {
const std::string key1 = "key1";
const std::string val1 = "val1";
// Before insert.
- EXPECT_THROW(conf->requiredProperty(key1), celeborn::CelebornUserError);
+ EXPECT_THROW(conf->requiredProperty(key1), CelebornUserError);
EXPECT_EQ(conf->optionalProperty(key1), ValueType());
// Insert.
- EXPECT_THROW(conf->setValue(key1, val1), celeborn::CelebornUserError);
+ EXPECT_THROW(conf->setValue(key1, val1), CelebornUserError);
EXPECT_TRUE(conf->registerProperty(key1, ValueType(val1)));
// After insert.
EXPECT_FALSE(conf->registerProperty(key1, ValueType(val1)));
- EXPECT_THROW(conf->requiredProperty(key1), celeborn::CelebornUserError);
+ EXPECT_THROW(conf->requiredProperty(key1), CelebornUserError);
EXPECT_EQ(conf->optionalProperty(key1), ValueType(val1));
// Update.
@@ -47,7 +49,7 @@ void testInsertToConf(TestedBaseConf* conf, bool isMutable =
true) {
if (isMutable) {
EXPECT_EQ(conf->setValue(key1, val1_1), ValueType(val1));
} else {
- EXPECT_THROW(conf->setValue(key1, val1_1), celeborn::CelebornUserError);
+ EXPECT_THROW(conf->setValue(key1, val1_1), CelebornUserError);
}
}
@@ -62,7 +64,7 @@ void testInitedConf(
// Test insert to init config.
const std::string val_1 = "val_1_random";
- EXPECT_THROW(conf->setValue(key, val_1), celeborn::CelebornUserError);
+ EXPECT_THROW(conf->setValue(key, val_1), CelebornUserError);
EXPECT_TRUE(conf->registerProperty(key, val_1));
// The init config has highest priority.
EXPECT_EQ(conf->optionalProperty(key), val);
@@ -75,7 +77,7 @@ void testInitedConf(
EXPECT_EQ(conf->optionalProperty(key), ValueType(val_1));
} else {
// Insert with setValue() would not work!
- EXPECT_THROW(conf->setValue(key, val_1), celeborn::CelebornUserError);
+ EXPECT_THROW(conf->setValue(key, val_1), CelebornUserError);
}
}
@@ -154,7 +156,7 @@ TEST(BaseConfTest,
registerToConfInitedWithMutableConfigFile) {
// The annotated kv would not be recorded.
EXPECT_THROW(
- conf->requiredProperty(annotatedKey), celeborn::CelebornUserError);
+ conf->requiredProperty(annotatedKey), CelebornUserError);
// Test init.
testInitedConf(conf.get(), true, key0, val0);
@@ -191,7 +193,7 @@ TEST(BaseConfTest,
registerToConfInitedWithImmutableConfigFile) {
// The annotated kv would not be recorded.
EXPECT_THROW(
- conf->requiredProperty(annotatedKey), celeborn::CelebornUserError);
+ conf->requiredProperty(annotatedKey), CelebornUserError);
// Test init.
testInitedConf(conf.get(), false, key0, val0);
diff --git a/cpp/celeborn/conf/tests/CelebornConfTest.cpp
b/cpp/celeborn/conf/tests/CelebornConfTest.cpp
index 058077191..41efdbc00 100644
--- a/cpp/celeborn/conf/tests/CelebornConfTest.cpp
+++ b/cpp/celeborn/conf/tests/CelebornConfTest.cpp
@@ -20,7 +20,9 @@
#include "celeborn/conf/CelebornConf.h"
-using namespace celeborn;
+using namespace celeborn::conf;
+
+using CelebornUserError = celeborn::utils::CelebornUserError;
using SECOND = std::chrono::seconds;
using MILLISENCOND = std::chrono::milliseconds;
diff --git a/cpp/celeborn/memory/ByteBuffer.cpp
b/cpp/celeborn/memory/ByteBuffer.cpp
index 0e049ea67..28ad87b15 100644
--- a/cpp/celeborn/memory/ByteBuffer.cpp
+++ b/cpp/celeborn/memory/ByteBuffer.cpp
@@ -18,6 +18,7 @@
#include "celeborn/memory/ByteBuffer.h"
namespace celeborn {
+namespace memory{
std::unique_ptr<WriteOnlyByteBuffer> ByteBuffer::createWriteOnly(
size_t initialCapacity,
bool isBigEndian) {
@@ -73,4 +74,5 @@ std::unique_ptr<folly::IOBuf> ByteBuffer::trimBuffer(
}
return std::move(data);
}
-} // namespace celeborn
\ No newline at end of file
+} // namespace memory
+} // namespace celeborn
diff --git a/cpp/celeborn/memory/ByteBuffer.h b/cpp/celeborn/memory/ByteBuffer.h
index 2c27ec9e8..763582d32 100644
--- a/cpp/celeborn/memory/ByteBuffer.h
+++ b/cpp/celeborn/memory/ByteBuffer.h
@@ -21,6 +21,7 @@
#include <folly/io/IOBuf.h>
namespace celeborn {
+namespace memory {
class ReadOnlyByteBuffer;
class WriteOnlyByteBuffer;
@@ -177,4 +178,5 @@ class WriteOnlyByteBuffer : public ByteBuffer {
private:
std::unique_ptr<folly::io::Appender> appender_;
};
+} // namespace memory
} // namespace celeborn
diff --git a/cpp/celeborn/memory/CMakeLists.txt
b/cpp/celeborn/memory/CMakeLists.txt
index ff6a331c7..c617d1f88 100644
--- a/cpp/celeborn/memory/CMakeLists.txt
+++ b/cpp/celeborn/memory/CMakeLists.txt
@@ -14,6 +14,7 @@
# limitations under the License.
add_library(
memory
+ STATIC
ByteBuffer.cpp)
diff --git a/cpp/celeborn/memory/tests/ByteBufferTest.cpp
b/cpp/celeborn/memory/tests/ByteBufferTest.cpp
index 26865add7..350b019a6 100644
--- a/cpp/celeborn/memory/tests/ByteBufferTest.cpp
+++ b/cpp/celeborn/memory/tests/ByteBufferTest.cpp
@@ -19,7 +19,7 @@
#include "celeborn/memory/ByteBuffer.h"
-using namespace celeborn;
+using namespace celeborn::memory;
namespace {
template <typename T>
diff --git a/cpp/celeborn/protocol/PartitionLocation.cpp
b/cpp/celeborn/protocol/PartitionLocation.cpp
index 5fc6e52c3..aee59586b 100644
--- a/cpp/celeborn/protocol/PartitionLocation.cpp
+++ b/cpp/celeborn/protocol/PartitionLocation.cpp
@@ -22,6 +22,7 @@
#include "celeborn/utils/Exceptions.h"
namespace celeborn {
+namespace protocol {
std::unique_ptr<StorageInfo> StorageInfo::fromPb(const PbStorageInfo& pb) {
auto result = std::make_unique<StorageInfo>();
result->type = static_cast<Type>(pb.type());
@@ -88,4 +89,5 @@ StatusCode toStatusCode(int32_t code) {
CELEBORN_CHECK(code <= StatusCode::TAIL);
return static_cast<StatusCode>(code);
}
+} // namespace protocol
} // namespace celeborn
diff --git a/cpp/celeborn/protocol/PartitionLocation.h
b/cpp/celeborn/protocol/PartitionLocation.h
index dc0f9ecd7..c485c1607 100644
--- a/cpp/celeborn/protocol/PartitionLocation.h
+++ b/cpp/celeborn/protocol/PartitionLocation.h
@@ -23,6 +23,7 @@
#include "celeborn/utils/Exceptions.h"
namespace celeborn {
+namespace protocol {
struct StorageInfo {
static std::unique_ptr<StorageInfo> fromPb(const PbStorageInfo& pb);
@@ -92,4 +93,5 @@ struct PartitionLocation {
static std::unique_ptr<PartitionLocation> fromPbWithoutPeer(
const PbPartitionLocation& pb);
};
+} // namespace protocol
} // namespace celeborn
diff --git a/cpp/celeborn/protocol/StatusCode.h
b/cpp/celeborn/protocol/StatusCode.h
index e5c4ee199..1050efccb 100644
--- a/cpp/celeborn/protocol/StatusCode.h
+++ b/cpp/celeborn/protocol/StatusCode.h
@@ -17,7 +17,7 @@
#pragma once
-namespace celeborn {
+namespace celeborn::protocol {
enum StatusCode {
// 1/0 Status
SUCCESS = 0,
@@ -92,4 +92,4 @@ enum StatusCode {
};
StatusCode toStatusCode(int32_t code);
-} // namespace celeborn
+} // namespace celeborn::protocol
diff --git a/cpp/celeborn/protocol/TransportMessage.cpp
b/cpp/celeborn/protocol/TransportMessage.cpp
index 351dac5b0..6335d0f44 100644
--- a/cpp/celeborn/protocol/TransportMessage.cpp
+++ b/cpp/celeborn/protocol/TransportMessage.cpp
@@ -25,7 +25,8 @@ TransportMessage::TransportMessage(MessageType type,
std::string&& payload)
messageTypeValue_ = type;
}
-TransportMessage::TransportMessage(std::unique_ptr<ReadOnlyByteBuffer> buf) {
+TransportMessage::TransportMessage(
+ std::unique_ptr<memory::ReadOnlyByteBuffer> buf) {
int messageTypeValue = buf->read<int32_t>();
int payloadLen = buf->read<int32_t>();
CELEBORN_CHECK_EQ(buf->remainingSize(), payloadLen);
@@ -35,15 +36,15 @@
TransportMessage::TransportMessage(std::unique_ptr<ReadOnlyByteBuffer> buf) {
payload_ = buf->readToString(payloadLen);
}
-std::unique_ptr<ReadOnlyByteBuffer> TransportMessage::toReadOnlyByteBuffer()
- const {
+std::unique_ptr<memory::ReadOnlyByteBuffer>
+TransportMessage::toReadOnlyByteBuffer() const {
int bufSize = payload_.size() + 4 + 4;
- auto buffer = ByteBuffer::createWriteOnly(bufSize);
+ auto buffer = memory::ByteBuffer::createWriteOnly(bufSize);
buffer->write<int>(messageTypeValue_);
buffer->write<int>(payload_.size());
buffer->writeFromString(payload_);
CELEBORN_CHECK_EQ(buffer->size(), bufSize);
- return ByteBuffer::toReadOnly(std::move(buffer));
+ return memory::ByteBuffer::toReadOnly(std::move(buffer));
}
} // namespace protocol
} // namespace celeborn
diff --git a/cpp/celeborn/protocol/TransportMessage.h
b/cpp/celeborn/protocol/TransportMessage.h
index 22a162eea..f6688b7b6 100644
--- a/cpp/celeborn/protocol/TransportMessage.h
+++ b/cpp/celeborn/protocol/TransportMessage.h
@@ -29,9 +29,9 @@ class TransportMessage {
public:
TransportMessage(MessageType type, std::string&& payload);
- TransportMessage(std::unique_ptr<ReadOnlyByteBuffer> buf);
+ TransportMessage(std::unique_ptr<memory::ReadOnlyByteBuffer> buf);
- std::unique_ptr<ReadOnlyByteBuffer> toReadOnlyByteBuffer() const;
+ std::unique_ptr<memory::ReadOnlyByteBuffer> toReadOnlyByteBuffer() const;
MessageType type() const {
return type_;
diff --git a/cpp/celeborn/protocol/tests/PartitionLocationTest.cpp
b/cpp/celeborn/protocol/tests/PartitionLocationTest.cpp
index ec318369b..20c6f40fd 100644
--- a/cpp/celeborn/protocol/tests/PartitionLocationTest.cpp
+++ b/cpp/celeborn/protocol/tests/PartitionLocationTest.cpp
@@ -20,7 +20,7 @@
#include "celeborn/proto/TransportMessagesCpp.pb.h"
#include "celeborn/protocol/PartitionLocation.h"
-using namespace celeborn;
+using namespace celeborn::protocol;
std::unique_ptr<PbStorageInfo> generateStorageInfoPb() {
auto pbStorageInfo = std::make_unique<PbStorageInfo>();
diff --git a/cpp/celeborn/protocol/tests/TransportMessageTest.cpp
b/cpp/celeborn/protocol/tests/TransportMessageTest.cpp
index c0f64c686..ca1a5202e 100644
--- a/cpp/celeborn/protocol/tests/TransportMessageTest.cpp
+++ b/cpp/celeborn/protocol/tests/TransportMessageTest.cpp
@@ -20,7 +20,6 @@
#include "celeborn/proto/TransportMessagesCpp.pb.h"
#include "celeborn/protocol/TransportMessage.h"
-using namespace celeborn;
using namespace celeborn::protocol;
TEST(TransportMessageTest, constructFromPayload) {
diff --git a/cpp/celeborn/utils/CMakeLists.txt
b/cpp/celeborn/utils/CMakeLists.txt
index f040d73b7..19f3efc44 100644
--- a/cpp/celeborn/utils/CMakeLists.txt
+++ b/cpp/celeborn/utils/CMakeLists.txt
@@ -14,13 +14,15 @@
# limitations under the License.
add_library(
utils
- ProcessBase.cpp StackTrace.cpp CelebornException.cpp Exceptions.cpp
flags.cpp)
+ STATIC
+ ProcessBase.cpp
+ StackTrace.cpp
+ CelebornException.cpp
+ Exceptions.cpp
+ flags.cpp)
target_link_libraries(
utils
- ${WANGLE}
- ${FIZZ}
- ${LIBSODIUM_LIBRARY}
${FOLLY_WITH_DEPENDENCIES}
${GLOG}
${GFLAGS_LIBRARIES}
diff --git a/cpp/celeborn/utils/CelebornException.cpp
b/cpp/celeborn/utils/CelebornException.cpp
index aa9c63e92..32637f0d0 100644
--- a/cpp/celeborn/utils/CelebornException.cpp
+++ b/cpp/celeborn/utils/CelebornException.cpp
@@ -24,6 +24,7 @@
#include <exception>
namespace celeborn {
+namespace utils {
std::exception_ptr toCelebornException(const std::exception_ptr& exceptionPtr)
{
try {
@@ -278,4 +279,5 @@ const char* CelebornException::State::what() const noexcept
{
}
}
+} // namespace utils
} // namespace celeborn
diff --git a/cpp/celeborn/utils/CelebornException.h
b/cpp/celeborn/utils/CelebornException.h
index a711ca78e..c7772021d 100644
--- a/cpp/celeborn/utils/CelebornException.h
+++ b/cpp/celeborn/utils/CelebornException.h
@@ -37,6 +37,7 @@
DECLARE_int32(celeborn_exception_user_stacktrace_rate_limit_ms);
DECLARE_int32(celeborn_exception_system_stacktrace_rate_limit_ms);
namespace celeborn {
+namespace utils {
namespace error_source {
using namespace folly::string_literals;
@@ -412,4 +413,5 @@ class ExceptionContextSetter {
private:
ExceptionContext prev_;
};
+} // namespace utils
} // namespace celeborn
diff --git a/cpp/celeborn/utils/CelebornUtils.h
b/cpp/celeborn/utils/CelebornUtils.h
index d42beb5f9..158b35944 100644
--- a/cpp/celeborn/utils/CelebornUtils.h
+++ b/cpp/celeborn/utils/CelebornUtils.h
@@ -22,6 +22,7 @@
#include "celeborn/utils/Exceptions.h"
namespace celeborn {
+namespace utils {
#define CELEBORN_STARTUP_LOG_PREFIX "[CELEBORN_STARTUP] "
#define CELEBORN_STARTUP_LOG(severity) \
LOG(severity) << CELEBORN_STARTUP_LOG_PREFIX
@@ -37,5 +38,5 @@ inline Timeout toTimeout(Duration duration) {
return std::chrono::duration_cast<Timeout>(duration);
}
+} // namespace utils
} // namespace celeborn
-
diff --git a/cpp/celeborn/utils/Exceptions.cpp
b/cpp/celeborn/utils/Exceptions.cpp
index 202d50d14..30069453a 100644
--- a/cpp/celeborn/utils/Exceptions.cpp
+++ b/cpp/celeborn/utils/Exceptions.cpp
@@ -18,9 +18,13 @@
#include "celeborn/utils/Exceptions.h"
-namespace celeborn::detail {
+namespace celeborn {
+namespace utils {
+namespace detail {
-CELEBORN_DEFINE_CHECK_FAIL_TEMPLATES(::celeborn::CelebornRuntimeError);
-CELEBORN_DEFINE_CHECK_FAIL_TEMPLATES(::celeborn::CelebornUserError);
+CELEBORN_DEFINE_CHECK_FAIL_TEMPLATES(::celeborn::utils::CelebornRuntimeError);
+CELEBORN_DEFINE_CHECK_FAIL_TEMPLATES(::celeborn::utils::CelebornUserError);
-} // namespace celeborn::detail
+} // namespace detail
+} // namespace utils
+} // namespace celeborn
diff --git a/cpp/celeborn/utils/Exceptions.h b/cpp/celeborn/utils/Exceptions.h
index 2a1e2fc98..e84646bfe 100644
--- a/cpp/celeborn/utils/Exceptions.h
+++ b/cpp/celeborn/utils/Exceptions.h
@@ -34,6 +34,7 @@
// #include "celeborn/utils/FmtStdFormatters.h"
namespace celeborn {
+namespace utils {
namespace detail {
struct CelebornCheckFailArgs {
@@ -117,7 +118,7 @@ struct CelebornCheckFailStringType<std::string> {
// and simply insert a function call for the linker to fix up, rather
// than emitting a definition of these templates into every
// translation unit they are used in.
-#define CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES(exception_type)
\
+#define CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES(exception_type) \
namespace detail { \
extern template void \
celebornCheckFail<exception_type, CompileTimeEmptyString>( \
@@ -133,7 +134,7 @@ struct CelebornCheckFailStringType<std::string> {
// Definitions corresponding to CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES. Should
// only be used in Exceptions.cpp.
-#define CELEBORN_DEFINE_CHECK_FAIL_TEMPLATES(exception_type)
\
+#define CELEBORN_DEFINE_CHECK_FAIL_TEMPLATES(exception_type) \
template void celebornCheckFail<exception_type, CompileTimeEmptyString>( \
const CelebornCheckFailArgs& args, CompileTimeEmptyString); \
template void celebornCheckFail<exception_type, const char*>( \
@@ -163,24 +164,24 @@ std::string errorMessage(fmt::string_view fmt, const
Args&... args) {
} // namespace detail
-#define _CELEBORN_THROW_IMPL( \
- exception, exprStr, errorSource, errorCode, isRetriable, ...) \
- { \
- /* GCC 9.2.1 doesn't accept this code with constexpr. */ \
- static const ::celeborn::detail::CelebornCheckFailArgs \
- celebornCheckFailArgs = { \
- __FILE__, \
- __LINE__, \
- __FUNCTION__, \
- exprStr, \
- errorSource, \
- errorCode, \
- isRetriable}; \
- auto message = ::celeborn::detail::errorMessage(__VA_ARGS__); \
- ::celeborn::detail::celebornCheckFail< \
- exception, \
- typename ::celeborn::detail::CelebornCheckFailStringType< \
- decltype(message)>::type>(celebornCheckFailArgs, message); \
+#define _CELEBORN_THROW_IMPL( \
+ exception, exprStr, errorSource, errorCode, isRetriable, ...) \
+ { \
+ /* GCC 9.2.1 doesn't accept this code with constexpr. */ \
+ static const ::celeborn::utils::detail::CelebornCheckFailArgs \
+ celebornCheckFailArgs = { \
+ __FILE__, \
+ __LINE__, \
+ __FUNCTION__, \
+ exprStr, \
+ errorSource, \
+ errorCode, \
+ isRetriable}; \
+ auto message = ::celeborn::utils::detail::errorMessage(__VA_ARGS__); \
+ ::celeborn::utils::detail::celebornCheckFail< \
+ exception, \
+ typename ::celeborn::utils::detail::CelebornCheckFailStringType< \
+ decltype(message)>::type>(celebornCheckFailArgs, message); \
}
#define _CELEBORN_CHECK_AND_THROW_IMPL(
\
@@ -193,16 +194,16 @@ std::string errorMessage(fmt::string_view fmt, const
Args&... args) {
#define _CELEBORN_THROW(exception, ...) \
_CELEBORN_THROW_IMPL(exception, "", ##__VA_ARGS__)
-CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES(::celeborn::CelebornRuntimeError);
+CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES(::celeborn::utils::CelebornRuntimeError);
-#define _CELEBORN_CHECK_IMPL(expr, exprStr, ...) \
- _CELEBORN_CHECK_AND_THROW_IMPL( \
- expr, \
- exprStr, \
- ::celeborn::CelebornRuntimeError, \
- ::celeborn::error_source::kErrorSourceRuntime.c_str(), \
- ::celeborn::error_code::kInvalidState.c_str(), \
- /* isRetriable */ false, \
+#define _CELEBORN_CHECK_IMPL(expr, exprStr, ...) \
+ _CELEBORN_CHECK_AND_THROW_IMPL( \
+ expr, \
+ exprStr, \
+ ::celeborn::utils::CelebornRuntimeError, \
+ ::celeborn::utils::error_source::kErrorSourceRuntime.c_str(), \
+ ::celeborn::utils::error_code::kInvalidState.c_str(), \
+ /* isRetriable */ false, \
##__VA_ARGS__)
// If the caller passes a custom message (4 *or more* arguments), we
@@ -238,14 +239,14 @@
CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES(::celeborn::CelebornRuntimeError);
_CELEBORN_CHECK_OP_HELPER( \
_CELEBORN_CHECK_IMPL, expr1, expr2, op, ##__VA_ARGS__)
-#define _CELEBORN_USER_CHECK_IMPL(expr, exprStr, ...) \
- _CELEBORN_CHECK_AND_THROW_IMPL( \
- expr, \
- exprStr, \
- ::celeborn::CelebornUserError, \
- ::celeborn::error_source::kErrorSourceUser.c_str(), \
- ::celeborn::error_code::kInvalidArgument.c_str(), \
- /* isRetriable */ false, \
+#define _CELEBORN_USER_CHECK_IMPL(expr, exprStr, ...) \
+ _CELEBORN_CHECK_AND_THROW_IMPL( \
+ expr, \
+ exprStr, \
+ ::celeborn::utils::CelebornUserError, \
+ ::celeborn::utils::error_source::kErrorSourceUser.c_str(), \
+ ::celeborn::utils::error_code::kInvalidArgument.c_str(), \
+ /* isRetriable */ false, \
##__VA_ARGS__)
#define _CELEBORN_USER_CHECK_OP(expr1, expr2, op, ...) \
@@ -274,48 +275,48 @@
CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES(::celeborn::CelebornRuntimeError);
#define CELEBORN_CHECK_OK(expr) \
do { \
- ::celeborn::Status _s = (expr); \
+ ::celeborn::utils::Status _s = (expr); \
_CELEBORN_CHECK_IMPL(_s.ok(), #expr, _s.toString()); \
} while (false)
-#define CELEBORN_UNSUPPORTED(...) \
- _CELEBORN_THROW( \
- ::celeborn::CelebornUserError, \
- ::celeborn::error_source::kErrorSourceUser.c_str(), \
- ::celeborn::error_code::kUnsupported.c_str(), \
- /* isRetriable */ false, \
+#define CELEBORN_UNSUPPORTED(...) \
+ _CELEBORN_THROW( \
+ ::celeborn::utils::CelebornUserError, \
+ ::celeborn::utils::error_source::kErrorSourceUser.c_str(), \
+ ::celeborn::utils::error_code::kUnsupported.c_str(), \
+ /* isRetriable */ false, \
##__VA_ARGS__)
-#define CELEBORN_ARITHMETIC_ERROR(...) \
- _CELEBORN_THROW( \
- ::celeborn::CelebornUserError, \
- ::celeborn::error_source::kErrorSourceUser.c_str(), \
- ::celeborn::error_code::kArithmeticError.c_str(), \
- /* isRetriable */ false, \
+#define CELEBORN_ARITHMETIC_ERROR(...) \
+ _CELEBORN_THROW( \
+ ::celeborn::utils::CelebornUserError, \
+ ::celeborn::utils::error_source::kErrorSourceUser.c_str(), \
+ ::celeborn::utils::error_code::kArithmeticError.c_str(), \
+ /* isRetriable */ false, \
##__VA_ARGS__)
-#define CELEBORN_SCHEMA_MISMATCH_ERROR(...) \
- _CELEBORN_THROW( \
- ::celeborn::CelebornUserError, \
- ::celeborn::error_source::kErrorSourceUser.c_str(), \
- ::celeborn::error_code::kSchemaMismatch.c_str(), \
- /* isRetriable */ false, \
+#define CELEBORN_SCHEMA_MISMATCH_ERROR(...) \
+ _CELEBORN_THROW( \
+ ::celeborn::utils::CelebornUserError, \
+ ::celeborn::utils::error_source::kErrorSourceUser.c_str(), \
+ ::celeborn::utils::error_code::kSchemaMismatch.c_str(), \
+ /* isRetriable */ false, \
##__VA_ARGS__)
-#define CELEBORN_FILE_NOT_FOUND_ERROR(...) \
- _CELEBORN_THROW( \
- ::celeborn::CelebornRuntimeError, \
- ::celeborn::error_source::kErrorSourceRuntime.c_str(), \
- ::celeborn::error_code::kFileNotFound.c_str(), \
- /* isRetriable */ false, \
+#define CELEBORN_FILE_NOT_FOUND_ERROR(...) \
+ _CELEBORN_THROW( \
+ ::celeborn::utils::CelebornRuntimeError, \
+ ::celeborn::utils::error_source::kErrorSourceRuntime.c_str(), \
+ ::celeborn::utils::error_code::kFileNotFound.c_str(), \
+ /* isRetriable */ false, \
##__VA_ARGS__)
-#define CELEBORN_UNREACHABLE(...) \
- _CELEBORN_THROW( \
- ::celeborn::CelebornRuntimeError, \
- ::celeborn::error_source::kErrorSourceRuntime.c_str(), \
- ::celeborn::error_code::kUnreachableCode.c_str(), \
- /* isRetriable */ false, \
+#define CELEBORN_UNREACHABLE(...) \
+ _CELEBORN_THROW( \
+ ::celeborn::utils::CelebornRuntimeError, \
+ ::celeborn::utils::error_source::kErrorSourceRuntime.c_str(), \
+ ::celeborn::utils::error_code::kUnreachableCode.c_str(), \
+ /* isRetriable */ false, \
##__VA_ARGS__)
#ifndef NDEBUG
@@ -341,15 +342,15 @@
CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES(::celeborn::CelebornRuntimeError);
#define CELEBORN_DCHECK_NOT_NULL(e, ...) CELEBORN_CHECK(true)
#endif
-#define CELEBORN_FAIL(...) \
- _CELEBORN_THROW( \
- ::celeborn::CelebornRuntimeError, \
- ::celeborn::error_source::kErrorSourceRuntime.c_str(), \
- ::celeborn::error_code::kInvalidState.c_str(), \
- /* isRetriable */ false, \
+#define CELEBORN_FAIL(...) \
+ _CELEBORN_THROW( \
+ ::celeborn::utils::CelebornRuntimeError, \
+ ::celeborn::utils::error_source::kErrorSourceRuntime.c_str(), \
+ ::celeborn::utils::error_code::kInvalidState.c_str(), \
+ /* isRetriable */ false, \
##__VA_ARGS__)
-CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES(::celeborn::CelebornUserError);
+CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES(::celeborn::utils::CelebornUserError);
// For all below macros, an additional message can be passed using a
// format string and arguments, as with `fmt::format`.
@@ -402,20 +403,21 @@
CELEBORN_DECLARE_CHECK_FAIL_TEMPLATES(::celeborn::CelebornUserError);
#define CELEBORN_USER_DCHECK_NOT_NULL(e, ...) CELEBORN_USER_CHECK(true)
#endif
-#define CELEBORN_USER_FAIL(...) \
- _CELEBORN_THROW( \
- ::celeborn::CelebornUserError, \
- ::celeborn::error_source::kErrorSourceUser.c_str(), \
- ::celeborn::error_code::kInvalidArgument.c_str(), \
- /* isRetriable */ false, \
+#define CELEBORN_USER_FAIL(...) \
+ _CELEBORN_THROW( \
+ ::celeborn::utils::CelebornUserError, \
+ ::celeborn::utils::error_source::kErrorSourceUser.c_str(), \
+ ::celeborn::utils::error_code::kInvalidArgument.c_str(), \
+ /* isRetriable */ false, \
##__VA_ARGS__)
-#define CELEBORN_NYI(...) \
- _CELEBORN_THROW( \
- ::celeborn::CelebornRuntimeError, \
- ::celeborn::error_source::kErrorSourceRuntime.c_str(), \
- ::celeborn::error_code::kNotImplemented.c_str(), \
- /* isRetriable */ false, \
+#define CELEBORN_NYI(...) \
+ _CELEBORN_THROW( \
+ ::celeborn::utils::CelebornRuntimeError, \
+ ::celeborn::utils::error_source::kErrorSourceRuntime.c_str(), \
+ ::celeborn::utils::error_code::kNotImplemented.c_str(), \
+ /* isRetriable */ false, \
##__VA_ARGS__)
+} // namespace utils
} // namespace celeborn
diff --git a/cpp/celeborn/utils/StackTrace.cpp
b/cpp/celeborn/utils/StackTrace.cpp
index de40851f7..09e0b48d4 100644
--- a/cpp/celeborn/utils/StackTrace.cpp
+++ b/cpp/celeborn/utils/StackTrace.cpp
@@ -41,7 +41,8 @@
#include <folly/fibers/FiberManager.h> // @manual
#endif
-namespace celeborn::utils {
+namespace celeborn {
+namespace utils {
StackTrace::StackTrace(int32_t skipFrames) {
create(skipFrames);
@@ -189,4 +190,5 @@ std::string StackTrace::demangle(const char* mangled) {
return folly::demangle(mangled).toStdString();
}
-} // namespace celeborn::utils
+} // namespace utils
+} // namespace celeborn
diff --git a/cpp/celeborn/utils/StackTrace.h b/cpp/celeborn/utils/StackTrace.h
index 9c9b98021..823c90e5b 100644
--- a/cpp/celeborn/utils/StackTrace.h
+++ b/cpp/celeborn/utils/StackTrace.h
@@ -23,7 +23,8 @@
#include <string>
#include <vector>
-namespace celeborn::utils {
+namespace celeborn {
+namespace utils {
///////////////////////////////////////////////////////////////////////////////
@@ -90,4 +91,5 @@ class StackTrace {
};
///////////////////////////////////////////////////////////////////////////////
-} // namespace celeborn::utils
+} // namespace utils
+} // namespace celeborn
diff --git a/cpp/celeborn/utils/tests/ExceptionTest.cpp
b/cpp/celeborn/utils/tests/ExceptionTest.cpp
index b1dc302ef..a8cc942fd 100644
--- a/cpp/celeborn/utils/tests/ExceptionTest.cpp
+++ b/cpp/celeborn/utils/tests/ExceptionTest.cpp
@@ -22,7 +22,7 @@
#include "celeborn/utils/Exceptions.h"
-using namespace celeborn;
+using namespace celeborn::utils;
struct Counter {
mutable int counter = 0;
@@ -109,11 +109,12 @@ void testExceptionTraceCollectionControl(bool
userException, bool enabled) {
false);
}
} catch (CelebornException& e) {
- SCOPED_TRACE(fmt::format(
- "enabled: {}, user flag: {}, sys flag: {}",
- enabled,
- FLAGS_celeborn_exception_user_stacktrace_enabled,
- FLAGS_celeborn_exception_system_stacktrace_enabled));
+ SCOPED_TRACE(
+ fmt::format(
+ "enabled: {}, user flag: {}, sys flag: {}",
+ enabled,
+ FLAGS_celeborn_exception_user_stacktrace_enabled,
+ FLAGS_celeborn_exception_system_stacktrace_enabled));
ASSERT_EQ(
userException, e.exceptionType() == CelebornException::Type::kUser);
ASSERT_EQ(enabled, e.stackTrace() != nullptr);
@@ -174,12 +175,13 @@ void testExceptionTraceCollectionRateControl(
false);
}
} catch (CelebornException& e) {
- SCOPED_TRACE(fmt::format(
- "userException: {}, hasRateLimit: {}, user limit: {}ms, sys limit:
{}ms",
- userException,
- hasRateLimit,
- FLAGS_celeborn_exception_user_stacktrace_rate_limit_ms,
- FLAGS_celeborn_exception_system_stacktrace_rate_limit_ms));
+ SCOPED_TRACE(
+ fmt::format(
+ "userException: {}, hasRateLimit: {}, user limit: {}ms, sys
limit: {}ms",
+ userException,
+ hasRateLimit,
+ FLAGS_celeborn_exception_user_stacktrace_rate_limit_ms,
+ FLAGS_celeborn_exception_system_stacktrace_rate_limit_ms));
ASSERT_EQ(
userException, e.exceptionType() == CelebornException::Type::kUser);
ASSERT_EQ(!hasRateLimit || ((iter % 2) == 0), e.stackTrace() != nullptr);
@@ -583,35 +585,36 @@ TEST(ExceptionTest, context) {
int* callCount;
};
- auto messageFunction = [](celeborn::CelebornException::Type exceptionType,
- void* untypedArg) {
- auto arg = static_cast<MessageFunctionArg*>(untypedArg);
- ++(*arg->callCount);
- switch (exceptionType) {
- case celeborn::CelebornException::Type::kUser:
- return fmt::format("User error: {}", arg->message);
- case celeborn::CelebornException::Type::kSystem:
- return fmt::format("System error: {}", arg->message);
- default:
- return fmt::format("Unexpected error type: {}", arg->message);
- }
- };
+ auto messageFunction =
+ [](celeborn::utils::CelebornException::Type exceptionType,
+ void* untypedArg) {
+ auto arg = static_cast<MessageFunctionArg*>(untypedArg);
+ ++(*arg->callCount);
+ switch (exceptionType) {
+ case celeborn::utils::CelebornException::Type::kUser:
+ return fmt::format("User error: {}", arg->message);
+ case celeborn::utils::CelebornException::Type::kSystem:
+ return fmt::format("System error: {}", arg->message);
+ default:
+ return fmt::format("Unexpected error type: {}", arg->message);
+ }
+ };
{
// Create multi-layer contexts with top level marked as essential.
MessageFunctionArg topLevelTroubleshootingAid{
"Top-level troubleshooting aid.", &callCount};
- celeborn::ExceptionContextSetter additionalContext(
+ celeborn::utils::ExceptionContextSetter additionalContext(
{.messageFunc = messageFunction, .arg = &topLevelTroubleshootingAid});
MessageFunctionArg midLevelTroubleshootingAid{
"Mid-level troubleshooting aid.", &callCount};
- celeborn::ExceptionContextSetter midLevelContext(
+ celeborn::utils::ExceptionContextSetter midLevelContext(
{messageFunction, &midLevelTroubleshootingAid});
MessageFunctionArg innerLevelTroubleshootingAid{
"Inner-level troubleshooting aid.", &callCount};
- celeborn::ExceptionContextSetter innerLevelContext(
+ celeborn::utils::ExceptionContextSetter innerLevelContext(
{messageFunction, &innerLevelTroubleshootingAid});
verifyCelebornException(
@@ -650,17 +653,17 @@ TEST(ExceptionTest, context) {
// Create multi-layer contexts with none marked as essential.
MessageFunctionArg topLevelTroubleshootingAid{
"Top-level troubleshooting aid.", &callCount};
- celeborn::ExceptionContextSetter additionalContext(
+ celeborn::utils::ExceptionContextSetter additionalContext(
{.messageFunc = messageFunction, .arg = &topLevelTroubleshootingAid});
MessageFunctionArg midLevelTroubleshootingAid{
"Mid-level troubleshooting aid.", &callCount};
- celeborn::ExceptionContextSetter midLevelContext(
+ celeborn::utils::ExceptionContextSetter midLevelContext(
{.messageFunc = messageFunction, .arg = &midLevelTroubleshootingAid});
MessageFunctionArg innerLevelTroubleshootingAid{
"Inner-level troubleshooting aid.", &callCount};
- celeborn::ExceptionContextSetter innerLevelContext(
+ celeborn::utils::ExceptionContextSetter innerLevelContext(
{messageFunction, &innerLevelTroubleshootingAid});
verifyCelebornException(
@@ -701,7 +704,8 @@ TEST(ExceptionTest, context) {
// Create a single layer of context. Context and top-level context are
// expected to be the same.
MessageFunctionArg debuggingInfo{"Debugging info.", &callCount};
- celeborn::ExceptionContextSetter context({messageFunction,
&debuggingInfo});
+ celeborn::utils::ExceptionContextSetter context(
+ {messageFunction, &debuggingInfo});
verifyCelebornException(
[&]() { CELEBORN_CHECK_EQ(1, 3); },
@@ -752,7 +756,7 @@ TEST(ExceptionTest, context) {
// With message function throwing an exception.
auto throwingMessageFunction =
- [](celeborn::CelebornException::Type /*exceptionType*/,
+ [](celeborn::utils::CelebornException::Type /*exceptionType*/,
void* untypedArg) -> std::string {
auto arg = static_cast<MessageFunctionArg*>(untypedArg);
++(*arg->callCount);
@@ -760,7 +764,7 @@ TEST(ExceptionTest, context) {
};
{
MessageFunctionArg debuggingInfo{"Debugging info.", &callCount};
- celeborn::ExceptionContextSetter context(
+ celeborn::utils::ExceptionContextSetter context(
{throwingMessageFunction, &debuggingInfo});
verifyCelebornException(
@@ -832,21 +836,23 @@ TEST(ExceptionTest, wrappedException) {
}
TEST(ExceptionTest, wrappedExceptionWithContext) {
- auto messageFunction = [](celeborn::CelebornException::Type exceptionType,
- void* untypedArg) {
- auto data = static_cast<char*>(untypedArg);
- switch (exceptionType) {
- case celeborn::CelebornException::Type::kUser:
- return fmt::format("User error: {}", data);
- case celeborn::CelebornException::Type::kSystem:
- return fmt::format("System error: {}", data);
- default:
- return fmt::format("Unexpected error type: {}", data);
- }
- };
+ auto messageFunction =
+ [](celeborn::utils::CelebornException::Type exceptionType,
+ void* untypedArg) {
+ auto data = static_cast<char*>(untypedArg);
+ switch (exceptionType) {
+ case celeborn::utils::CelebornException::Type::kUser:
+ return fmt::format("User error: {}", data);
+ case celeborn::utils::CelebornException::Type::kSystem:
+ return fmt::format("System error: {}", data);
+ default:
+ return fmt::format("Unexpected error type: {}", data);
+ }
+ };
std::string data = "lakes";
- celeborn::ExceptionContextSetter context({messageFunction, data.data()});
+ celeborn::utils::ExceptionContextSetter context(
+ {messageFunction, data.data()});
try {
throw std::invalid_argument("This is a test.");
@@ -873,7 +879,7 @@ TEST(ExceptionTest, wrappedExceptionWithContext) {
}
std::string innerData = "mountains";
- celeborn::ExceptionContextSetter innerContext(
+ celeborn::utils::ExceptionContextSetter innerContext(
{messageFunction, innerData.data()});
try {
