From 3aa185412969f14ad4f7cb3d302458bfee365b13 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Wed, 10 Dec 2025 16:29:29 +0000 Subject: [PATCH 01/15] refactor: Add writing command to etl::SystemState (#2842) --- src/etl/ETLService.cpp | 20 ++++--- src/etl/ETLService.hpp | 2 +- src/etl/SystemState.hpp | 23 +++++++- src/etl/impl/LedgerPublisher.hpp | 17 +++++- src/etl/impl/Loading.cpp | 2 +- tests/unit/etl/ETLServiceTests.cpp | 78 +++++++++++++++++++++++-- tests/unit/etl/LedgerPublisherTests.cpp | 4 +- tests/unit/etl/LoadingTests.cpp | 50 ++++++++++++++++ 8 files changed, 175 insertions(+), 21 deletions(-) diff --git a/src/etl/ETLService.cpp b/src/etl/ETLService.cpp index 6f8c0d4ee..595411cb4 100644 --- a/src/etl/ETLService.cpp +++ b/src/etl/ETLService.cpp @@ -348,14 +348,21 @@ ETLService::startMonitor(uint32_t seq) { monitor_ = monitorProvider_->make(ctx_, backend_, ledgers_, seq); + systemStateWriteCommandSubscription_ = + state_->writeCommandSignal.connect([this](SystemState::WriteCommand command) { + switch (command) { + case etl::SystemState::WriteCommand::StartWriting: + attemptTakeoverWriter(); + break; + case etl::SystemState::WriteCommand::StopWriting: + giveUpWriter(); + break; + } + }); + monitorNewSeqSubscription_ = monitor_->subscribeToNewSequence([this](uint32_t seq) { LOG(log_.info()) << "ETLService (via Monitor) got new seq from db: " << seq; - if (state_->writeConflict) { - LOG(log_.info()) << "Got a write conflict; Giving up writer seat immediately"; - giveUpWriter(); - } - if (not state_->isWriting) { auto const diff = data::synchronousAndRetryOnTimeout([this, seq](auto yield) { return backend_->fetchLedgerDiff(seq, yield); @@ -371,7 +378,7 @@ ETLService::startMonitor(uint32_t seq) monitorDbStalledSubscription_ = monitor_->subscribeToDbStalled([this]() { LOG(log_.warn()) << "ETLService received DbStalled signal from Monitor"; if (not state_->isStrictReadonly and not state_->isWriting) - attemptTakeoverWriter(); + state_->writeCommandSignal(SystemState::WriteCommand::StartWriting); }); monitor_->run(); @@ -404,7 +411,6 @@ ETLService::giveUpWriter() { ASSERT(not state_->isStrictReadonly, "This should only happen on writer nodes"); state_->isWriting = false; - state_->writeConflict = false; taskMan_ = nullptr; } diff --git a/src/etl/ETLService.hpp b/src/etl/ETLService.hpp index 45185d4be..689d4d14d 100644 --- a/src/etl/ETLService.hpp +++ b/src/etl/ETLService.hpp @@ -74,7 +74,6 @@ #include #include #include -#include namespace etl { @@ -117,6 +116,7 @@ class ETLService : public ETLServiceInterface { boost::signals2::scoped_connection monitorNewSeqSubscription_; boost::signals2::scoped_connection monitorDbStalledSubscription_; + boost::signals2::scoped_connection systemStateWriteCommandSubscription_; std::optional> mainLoop_; diff --git a/src/etl/SystemState.hpp b/src/etl/SystemState.hpp index 7f841665f..188d53fcb 100644 --- a/src/etl/SystemState.hpp +++ b/src/etl/SystemState.hpp @@ -23,7 +23,8 @@ #include "util/prometheus/Label.hpp" #include "util/prometheus/Prometheus.hpp" -#include +#include +#include namespace etl { @@ -50,8 +51,24 @@ struct SystemState { "Whether the process is writing to the database" ); - std::atomic_bool isStopping = false; /**< @brief Whether the software is stopping. */ - std::atomic_bool writeConflict = false; /**< @brief Whether a write conflict was detected. */ + /** + * @brief Commands for controlling the ETL writer state. + * + * These commands are emitted via writeCommandSignal to coordinate writer state transitions across components. + */ + enum class WriteCommand { + StartWriting, /**< Request to attempt taking over as the ETL writer */ + StopWriting /**< Request to give up the ETL writer role (e.g., due to write conflict) */ + }; + + /** + * @brief Signal for coordinating ETL writer state transitions. + * + * This signal allows components to request changes to the writer state without direct coupling. + * - Emitted with StartWriting when database stalls and node should attempt to become writer + * - Emitted with StopWriting when write conflicts are detected + */ + boost::signals2::signal writeCommandSignal; /** * @brief Whether clio detected an amendment block. diff --git a/src/etl/impl/LedgerPublisher.hpp b/src/etl/impl/LedgerPublisher.hpp index 0b48ca3f6..1db50299b 100644 --- a/src/etl/impl/LedgerPublisher.hpp +++ b/src/etl/impl/LedgerPublisher.hpp @@ -45,6 +45,7 @@ #include #include +#include #include #include #include @@ -76,6 +77,8 @@ class LedgerPublisher : public LedgerPublisherInterface { util::async::AnyStrand publishStrand_; + std::atomic_bool stop_{false}; + std::shared_ptr backend_; std::shared_ptr subscriptions_; std::reference_wrapper state_; // shared state for ETL @@ -125,7 +128,7 @@ public: { LOG(log_.info()) << "Attempting to publish ledger = " << ledgerSequence; size_t numAttempts = 0; - while (not state_.get().isStopping) { + while (not stop_) { auto range = backend_->hardFetchLedgerRangeNoThrow(); if (!range || range->maxSequence < ledgerSequence) { @@ -258,6 +261,18 @@ public: return *lastPublishedSequence_.lock(); } + /** + * @brief Stops publishing + * + * @note This is a basic implementation to satisfy tests. This will be improved in + * https://github.com/XRPLF/clio/issues/2833 + */ + void + stop() + { + stop_ = true; + } + private: void setLastClose(std::chrono::time_point lastCloseTime) diff --git a/src/etl/impl/Loading.cpp b/src/etl/impl/Loading.cpp index 59d2d0a9c..9c378f253 100644 --- a/src/etl/impl/Loading.cpp +++ b/src/etl/impl/Loading.cpp @@ -75,7 +75,7 @@ Loader::load(model::LedgerData const& data) << "; took " << duration << "ms"; if (not success) { - state_->writeConflict = true; + state_->writeCommandSignal(SystemState::WriteCommand::StopWriting); LOG(log_.warn()) << "Another node wrote a ledger into the DB - we have a write conflict"; return std::unexpected(LoaderError::WriteConflict); } diff --git a/tests/unit/etl/ETLServiceTests.cpp b/tests/unit/etl/ETLServiceTests.cpp index 253009459..59a570a44 100644 --- a/tests/unit/etl/ETLServiceTests.cpp +++ b/tests/unit/etl/ETLServiceTests.cpp @@ -216,6 +216,10 @@ protected: std::shared_ptr> monitorProvider_ = std::make_shared>(); std::shared_ptr systemState_ = std::make_shared(); + testing::StrictMock> mockWriteSignalCommandCallback_; + boost::signals2::scoped_connection writeCommandConnection_{ + systemState_->writeCommandSignal.connect(mockWriteSignalCommandCallback_.AsStdFunction()) + }; etl::ETLService service_{ ctx_, @@ -370,13 +374,13 @@ TEST_F(ETLServiceTests, HandlesWriteConflictInMonitorSubscription) EXPECT_CALL(*cacheLoader_, load(kSEQ)); service_.run(); - systemState_->writeConflict = true; + writeCommandConnection_.disconnect(); + systemState_->writeCommandSignal(etl::SystemState::WriteCommand::StopWriting); EXPECT_CALL(*publisher_, publish(kSEQ + 1, testing::_, testing::_)); ASSERT_TRUE(capturedCallback); capturedCallback(kSEQ + 1); - EXPECT_FALSE(systemState_->writeConflict); EXPECT_FALSE(systemState_->isWriting); } @@ -447,6 +451,8 @@ TEST_F(ETLServiceTests, AttemptTakeoverWriter) EXPECT_CALL(*taskManagerProvider_, make(testing::_, testing::_, kSEQ + 1, testing::_)) .WillOnce(testing::Return(std::move(mockTaskManager))); + EXPECT_CALL(mockWriteSignalCommandCallback_, Call(etl::SystemState::WriteCommand::StartWriting)); + ASSERT_TRUE(capturedDbStalledCallback); capturedDbStalledCallback(); @@ -477,15 +483,15 @@ TEST_F(ETLServiceTests, GiveUpWriterAfterWriteConflict) service_.run(); systemState_->isWriting = true; - systemState_->writeConflict = true; // got a write conflict along the way + writeCommandConnection_.disconnect(); + systemState_->writeCommandSignal(etl::SystemState::WriteCommand::StopWriting); EXPECT_CALL(*publisher_, publish(kSEQ + 1, testing::_, testing::_)); ASSERT_TRUE(capturedCallback); capturedCallback(kSEQ + 1); - EXPECT_FALSE(systemState_->isWriting); // gives up writing - EXPECT_FALSE(systemState_->writeConflict); // and removes write conflict flag + EXPECT_FALSE(systemState_->isWriting); // gives up writing } TEST_F(ETLServiceTests, CancelledLoadInitialLedger) @@ -539,3 +545,65 @@ TEST_F(ETLServiceTests, RunStopsIfInitialLoadIsCancelledByBalancer) EXPECT_FALSE(service_.isAmendmentBlocked()); EXPECT_FALSE(service_.isCorruptionDetected()); } + +TEST_F(ETLServiceTests, DbStalledDoesNotTriggerSignalWhenStrictReadonly) +{ + auto mockMonitor = std::make_unique>(); + auto& mockMonitorRef = *mockMonitor; + std::function capturedDbStalledCallback; + + EXPECT_CALL(*monitorProvider_, make).WillOnce([&mockMonitor](auto, auto, auto, auto, auto) { + return std::move(mockMonitor); + }); + EXPECT_CALL(mockMonitorRef, subscribeToNewSequence); + EXPECT_CALL(mockMonitorRef, subscribeToDbStalled).WillOnce([&capturedDbStalledCallback](auto callback) { + capturedDbStalledCallback = callback; + return boost::signals2::scoped_connection{}; + }); + EXPECT_CALL(mockMonitorRef, run); + + EXPECT_CALL(*backend_, hardFetchLedgerRange) + .WillRepeatedly(testing::Return(data::LedgerRange{.minSequence = 1, .maxSequence = kSEQ})); + EXPECT_CALL(*ledgers_, getMostRecent()).WillOnce(testing::Return(kSEQ)); + EXPECT_CALL(*cacheLoader_, load(kSEQ)); + + service_.run(); + systemState_->isStrictReadonly = true; // strict readonly mode + systemState_->isWriting = false; + + // No signal should be emitted because node is in strict readonly mode + + ASSERT_TRUE(capturedDbStalledCallback); + capturedDbStalledCallback(); +} + +TEST_F(ETLServiceTests, DbStalledDoesNotTriggerSignalWhenAlreadyWriting) +{ + auto mockMonitor = std::make_unique>(); + auto& mockMonitorRef = *mockMonitor; + std::function capturedDbStalledCallback; + + EXPECT_CALL(*monitorProvider_, make).WillOnce([&mockMonitor](auto, auto, auto, auto, auto) { + return std::move(mockMonitor); + }); + EXPECT_CALL(mockMonitorRef, subscribeToNewSequence); + EXPECT_CALL(mockMonitorRef, subscribeToDbStalled).WillOnce([&capturedDbStalledCallback](auto callback) { + capturedDbStalledCallback = callback; + return boost::signals2::scoped_connection{}; + }); + EXPECT_CALL(mockMonitorRef, run); + + EXPECT_CALL(*backend_, hardFetchLedgerRange) + .WillRepeatedly(testing::Return(data::LedgerRange{.minSequence = 1, .maxSequence = kSEQ})); + EXPECT_CALL(*ledgers_, getMostRecent()).WillOnce(testing::Return(kSEQ)); + EXPECT_CALL(*cacheLoader_, load(kSEQ)); + + service_.run(); + systemState_->isStrictReadonly = false; + systemState_->isWriting = true; // already writing + + // No signal should be emitted because node is already writing + + ASSERT_TRUE(capturedDbStalledCallback); + capturedDbStalledCallback(); +} diff --git a/tests/unit/etl/LedgerPublisherTests.cpp b/tests/unit/etl/LedgerPublisherTests.cpp index e4d422a9d..5b3c73d0f 100644 --- a/tests/unit/etl/LedgerPublisherTests.cpp +++ b/tests/unit/etl/LedgerPublisherTests.cpp @@ -216,15 +216,14 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerHeaderCloseTimeGreaterThanNow) TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqStopIsTrue) { auto dummyState = etl::SystemState{}; - dummyState.isStopping = true; auto publisher = impl::LedgerPublisher(ctx, backend_, mockSubscriptionManagerPtr, dummyState); + publisher.stop(); EXPECT_FALSE(publisher.publish(kSEQ, {})); } TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqMaxAttempt) { auto dummyState = etl::SystemState{}; - dummyState.isStopping = false; auto publisher = impl::LedgerPublisher(ctx, backend_, mockSubscriptionManagerPtr, dummyState); static constexpr auto kMAX_ATTEMPT = 2; @@ -238,7 +237,6 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqMaxAttempt) TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqStopIsFalse) { auto dummyState = etl::SystemState{}; - dummyState.isStopping = false; auto publisher = impl::LedgerPublisher(ctx, backend_, mockSubscriptionManagerPtr, dummyState); LedgerRange const range{.minSequence = kSEQ, .maxSequence = kSEQ}; diff --git a/tests/unit/etl/LoadingTests.cpp b/tests/unit/etl/LoadingTests.cpp index 143f915a2..6631fde73 100644 --- a/tests/unit/etl/LoadingTests.cpp +++ b/tests/unit/etl/LoadingTests.cpp @@ -188,3 +188,53 @@ TEST_F(LoadingAssertTest, LoadInitialLedgerHasDataInDB) EXPECT_CLIO_ASSERT_FAIL({ [[maybe_unused]] auto unused = loader_.loadInitialLedger(data); }); } + +TEST_F(LoadingTests, LoadWriteConflictEmitsStopWritingSignal) +{ + state_->isWriting = true; // writer is active + auto const data = createTestData(); + testing::StrictMock> mockSignalCallback; + + auto connection = state_->writeCommandSignal.connect(mockSignalCallback.AsStdFunction()); + + EXPECT_CALL(*mockRegistryPtr_, dispatch(data)); + EXPECT_CALL(*backend_, doFinishWrites()).WillOnce(testing::Return(false)); // simulate write conflict + EXPECT_CALL(mockSignalCallback, Call(etl::SystemState::WriteCommand::StopWriting)); + + auto result = loader_.load(data); + EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.error(), etl::LoaderError::WriteConflict); +} + +TEST_F(LoadingTests, LoadSuccessDoesNotEmitSignal) +{ + state_->isWriting = true; // writer is active + auto const data = createTestData(); + testing::StrictMock> mockSignalCallback; + + auto connection = state_->writeCommandSignal.connect(mockSignalCallback.AsStdFunction()); + + EXPECT_CALL(*mockRegistryPtr_, dispatch(data)); + EXPECT_CALL(*backend_, doFinishWrites()).WillOnce(testing::Return(true)); // success + // No signal should be emitted on success + + auto result = loader_.load(data); + EXPECT_TRUE(result.has_value()); +} + +TEST_F(LoadingTests, LoadWhenNotWritingDoesNotCheckConflict) +{ + state_->isWriting = false; // not a writer + auto const data = createTestData(); + testing::StrictMock> mockSignalCallback; + + auto connection = state_->writeCommandSignal.connect(mockSignalCallback.AsStdFunction()); + + EXPECT_CALL(*mockRegistryPtr_, dispatch(data)); + // doFinishWrites should not be called when not writing + EXPECT_CALL(*backend_, doFinishWrites()).Times(0); + // No signal should be emitted + + auto result = loader_.load(data); + EXPECT_TRUE(result.has_value()); +} From e9b98cf5b365cd523fe75a0c5fd5044e6ef5701c Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 11 Dec 2025 09:51:03 +0000 Subject: [PATCH 02/15] style: clang-tidy auto fixes (#2848) Co-authored-by: godexsoft <385326+godexsoft@users.noreply.github.com> --- tests/unit/etl/LoadingTests.cpp | 1 + .../handlers/AccountMPTokenIssuancesTests.cpp | 46 +++++++++++++------ 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/tests/unit/etl/LoadingTests.cpp b/tests/unit/etl/LoadingTests.cpp index 6631fde73..1554d7d1b 100644 --- a/tests/unit/etl/LoadingTests.cpp +++ b/tests/unit/etl/LoadingTests.cpp @@ -19,6 +19,7 @@ #include "data/Types.hpp" #include "etl/InitialLoadObserverInterface.hpp" +#include "etl/LoaderInterface.hpp" #include "etl/Models.hpp" #include "etl/RegistryInterface.hpp" #include "etl/SystemState.hpp" diff --git a/tests/unit/rpc/handlers/AccountMPTokenIssuancesTests.cpp b/tests/unit/rpc/handlers/AccountMPTokenIssuancesTests.cpp index 09af77b63..5e1a045e9 100644 --- a/tests/unit/rpc/handlers/AccountMPTokenIssuancesTests.cpp +++ b/tests/unit/rpc/handlers/AccountMPTokenIssuancesTests.cpp @@ -985,13 +985,13 @@ static auto generateSingleFlagTests() { return std::vector{ - {"Locked", ripple::lsfMPTLocked, "mpt_locked"}, - {"CanLock", ripple::lsfMPTCanLock, "mpt_can_lock"}, - {"RequireAuth", ripple::lsfMPTRequireAuth, "mpt_require_auth"}, - {"CanEscrow", ripple::lsfMPTCanEscrow, "mpt_can_escrow"}, - {"CanTrade", ripple::lsfMPTCanTrade, "mpt_can_trade"}, - {"CanTransfer", ripple::lsfMPTCanTransfer, "mpt_can_transfer"}, - {"CanClawback", ripple::lsfMPTCanClawback, "mpt_can_clawback"}, + {.testName = "Locked", .flag = ripple::lsfMPTLocked, .expectedJsonKey = "mpt_locked"}, + {.testName = "CanLock", .flag = ripple::lsfMPTCanLock, .expectedJsonKey = "mpt_can_lock"}, + {.testName = "RequireAuth", .flag = ripple::lsfMPTRequireAuth, .expectedJsonKey = "mpt_require_auth"}, + {.testName = "CanEscrow", .flag = ripple::lsfMPTCanEscrow, .expectedJsonKey = "mpt_can_escrow"}, + {.testName = "CanTrade", .flag = ripple::lsfMPTCanTrade, .expectedJsonKey = "mpt_can_trade"}, + {.testName = "CanTransfer", .flag = ripple::lsfMPTCanTransfer, .expectedJsonKey = "mpt_can_transfer"}, + {.testName = "CanClawback", .flag = ripple::lsfMPTCanClawback, .expectedJsonKey = "mpt_can_clawback"}, }; } @@ -1059,14 +1059,30 @@ static auto generateSingleMutableFlagTests() { return std::vector{ - {"CanMutateCanLock", ripple::lsmfMPTCanMutateCanLock, "mpt_can_mutate_can_lock"}, - {"CanMutateRequireAuth", ripple::lsmfMPTCanMutateRequireAuth, "mpt_can_mutate_require_auth"}, - {"CanMutateCanEscrow", ripple::lsmfMPTCanMutateCanEscrow, "mpt_can_mutate_can_escrow"}, - {"CanMutateCanTrade", ripple::lsmfMPTCanMutateCanTrade, "mpt_can_mutate_can_trade"}, - {"CanMutateCanTransfer", ripple::lsmfMPTCanMutateCanTransfer, "mpt_can_mutate_can_transfer"}, - {"CanMutateCanClawback", ripple::lsmfMPTCanMutateCanClawback, "mpt_can_mutate_can_clawback"}, - {"CanMutateMetadata", ripple::lsmfMPTCanMutateMetadata, "mpt_can_mutate_metadata"}, - {"CanMutateTransferFee", ripple::lsmfMPTCanMutateTransferFee, "mpt_can_mutate_transfer_fee"}, + {.testName = "CanMutateCanLock", + .mutableFlag = ripple::lsmfMPTCanMutateCanLock, + .expectedJsonKey = "mpt_can_mutate_can_lock"}, + {.testName = "CanMutateRequireAuth", + .mutableFlag = ripple::lsmfMPTCanMutateRequireAuth, + .expectedJsonKey = "mpt_can_mutate_require_auth"}, + {.testName = "CanMutateCanEscrow", + .mutableFlag = ripple::lsmfMPTCanMutateCanEscrow, + .expectedJsonKey = "mpt_can_mutate_can_escrow"}, + {.testName = "CanMutateCanTrade", + .mutableFlag = ripple::lsmfMPTCanMutateCanTrade, + .expectedJsonKey = "mpt_can_mutate_can_trade"}, + {.testName = "CanMutateCanTransfer", + .mutableFlag = ripple::lsmfMPTCanMutateCanTransfer, + .expectedJsonKey = "mpt_can_mutate_can_transfer"}, + {.testName = "CanMutateCanClawback", + .mutableFlag = ripple::lsmfMPTCanMutateCanClawback, + .expectedJsonKey = "mpt_can_mutate_can_clawback"}, + {.testName = "CanMutateMetadata", + .mutableFlag = ripple::lsmfMPTCanMutateMetadata, + .expectedJsonKey = "mpt_can_mutate_metadata"}, + {.testName = "CanMutateTransferFee", + .mutableFlag = ripple::lsmfMPTCanMutateTransferFee, + .expectedJsonKey = "mpt_can_mutate_transfer_fee"}, }; } From f2c4275f613800116ad65f139f4f4d3e94618d97 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Fri, 12 Dec 2025 16:39:53 +0000 Subject: [PATCH 03/15] ci: Put debian package to release (#2850) --- .github/workflows/build.yml | 18 ------------------ .github/workflows/nightly.yml | 20 +++++++++++++++++++- .github/workflows/release.yml | 20 +++++++++++++++++++- .github/workflows/reusable-release.yml | 13 +++++++++---- 4 files changed, 47 insertions(+), 24 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bd5d08699..7c86f44d7 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -92,24 +92,6 @@ jobs: secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} - package: - name: Build packages - - uses: ./.github/workflows/reusable-build.yml - with: - runs_on: heavy - container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' - conan_profile: gcc - build_type: Release - download_ccache: true - upload_ccache: false - code_coverage: false - static: true - upload_clio_server: false - package: true - targets: package - analyze_build_time: false - check_config: name: Check Config Description needs: build-and-test diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 49e060c00..b1529190c 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -68,6 +68,24 @@ jobs: download_ccache: false upload_ccache: false + package: + name: Build debian package + + uses: ./.github/workflows/reusable-build.yml + with: + runs_on: heavy + container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' + conan_profile: gcc + build_type: Release + download_ccache: false + upload_ccache: false + code_coverage: false + static: true + upload_clio_server: false + package: true + targets: package + analyze_build_time: false + analyze_build_time: name: Analyze Build Time @@ -109,7 +127,7 @@ jobs: echo "date=$(date +'%Y%m%d')" >> $GITHUB_OUTPUT nightly_release: - needs: [build-and-test, get_date] + needs: [build-and-test, package, get_date] uses: ./.github/workflows/reusable-release.yml with: delete_pattern: "nightly-*" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 76bc0172a..158107a42 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -45,8 +45,26 @@ jobs: upload_ccache: false expected_version: ${{ github.event_name == 'push' && github.ref_name || '' }} + package: + name: Build debian package + + uses: ./.github/workflows/reusable-build.yml + with: + runs_on: heavy + container: '{ "image": "ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f" }' + conan_profile: gcc + build_type: Release + download_ccache: false + upload_ccache: false + code_coverage: false + static: true + upload_clio_server: false + package: true + targets: package + analyze_build_time: false + release: - needs: build-and-test + needs: [build-and-test, package] uses: ./.github/workflows/reusable-release.yml with: delete_pattern: "" diff --git a/.github/workflows/reusable-release.yml b/.github/workflows/reusable-release.yml index 757774c39..9f068275b 100644 --- a/.github/workflows/reusable-release.yml +++ b/.github/workflows/reusable-release.yml @@ -69,6 +69,14 @@ jobs: path: release_artifacts pattern: clio_server_* + - name: Prepare release artifacts + run: .github/scripts/prepare-release-artifacts.sh release_artifacts + + - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + with: + path: release_artifacts + pattern: clio_deb_package_* + - name: Create release notes env: RELEASE_HEADER: ${{ inputs.header }} @@ -86,9 +94,6 @@ jobs: git-cliff "${BASE_COMMIT}..HEAD" --ignore-tags "nightly|-b|-rc" cat CHANGELOG.md >> "${RUNNER_TEMP}/release_notes.md" - - name: Prepare release artifacts - run: .github/scripts/prepare-release-artifacts.sh release_artifacts - - name: Upload release notes uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 with: @@ -122,4 +127,4 @@ jobs: --target "${GITHUB_SHA}" \ ${DRAFT_OPTION} \ --notes-file "${RUNNER_TEMP}/release_notes.md" \ - ./release_artifacts/clio_server* + ./release_artifacts/clio_* From 488bb05d2268a3c4c859bacfcf5d93be5ac9fc86 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Fri, 12 Dec 2025 16:40:00 +0000 Subject: [PATCH 04/15] chore: Add a script to regenerate conan lockfile (#2849) --- .github/scripts/conan/regenerate_lockfile.sh | 25 ++++++++++++++++++++ .github/workflows/upload-conan-deps.yml | 2 ++ docs/build-clio.md | 20 ++-------------- 3 files changed, 29 insertions(+), 18 deletions(-) create mode 100755 .github/scripts/conan/regenerate_lockfile.sh diff --git a/.github/scripts/conan/regenerate_lockfile.sh b/.github/scripts/conan/regenerate_lockfile.sh new file mode 100755 index 000000000..2024c80d6 --- /dev/null +++ b/.github/scripts/conan/regenerate_lockfile.sh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash + +set -ex + +TEMP_DIR=$(mktemp -d) +trap "rm -rf $TEMP_DIR" EXIT + +echo "Using temporary CONAN_HOME: $TEMP_DIR" + +# We use a temporary Conan home to avoid polluting the user's existing Conan +# configuration and to not use local cache (which leads to non-reproducible lockfiles). +export CONAN_HOME="$TEMP_DIR" + +# Ensure that the xrplf remote is the first to be consulted, so any recipes we +# patched are used. We also add it there to not created huge diff when the +# official Conan Center Index is updated. +conan remote add --force --index 0 xrplf https://conan.ripplex.io + +# Delete any existing lockfile. +rm -f conan.lock + +# Create a new lockfile that is compatible with macOS. +# It should also work on Linux. +conan lock create . \ + --profile:all=.github/scripts/conan/apple-clang-17.profile diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index 178e80d6c..dd31d3bc5 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -22,6 +22,7 @@ on: - .github/actions/conan/action.yml - ".github/scripts/conan/**" + - "!.github/scripts/conan/regenerate_lockfile.sh" - conanfile.py - conan.lock @@ -32,6 +33,7 @@ on: - .github/actions/conan/action.yml - ".github/scripts/conan/**" + - "!.github/scripts/conan/regenerate_lockfile.sh" - conanfile.py - conan.lock diff --git a/docs/build-clio.md b/docs/build-clio.md index 51c6822b5..791bc49d3 100644 --- a/docs/build-clio.md +++ b/docs/build-clio.md @@ -97,30 +97,14 @@ Now you should be able to download the prebuilt dependencies (including `xrpl` p #### Conan lockfile -To achieve reproducible dependencies, we use [Conan lockfile](https://docs.conan.io/2/tutorial/versioning/lockfiles.html). +To achieve reproducible dependencies, we use a [Conan lockfile](https://docs.conan.io/2/tutorial/versioning/lockfiles.html). The `conan.lock` file in the repository contains a "snapshot" of the current dependencies. It is implicitly used when running `conan` commands, you don't need to specify it. You have to update this file every time you add a new dependency or change a revision or version of an existing dependency. -> [!NOTE] -> Conan uses local cache by default when creating a lockfile. -> -> To ensure, that lockfile creation works the same way on all developer machines, you should clear the local cache before creating a new lockfile. - -To create a new lockfile, run the following commands in the repository root: - -```bash -conan remove '*' --confirm -rm conan.lock -# This ensure that xrplf remote is the first to be consulted -conan remote add --force --index 0 xrplf https://conan.ripplex.io -conan lock create . -``` - -> [!NOTE] -> If some dependencies are exclusive for some OS, you may need to run the last command for them adding `--profile:all `. +To update a lockfile, run from the repository root: `./.github/scripts/conan/regenerate_lockfile.sh` ## Building Clio From f451996944408ef186fe4d243f7050a4c8059df2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Dec 2025 12:34:55 +0000 Subject: [PATCH 05/15] ci: [DEPENDABOT] Bump tj-actions/changed-files from 46.0.5 to 47.0.1 (#2853) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/update-docker-ci.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/update-docker-ci.yml b/.github/workflows/update-docker-ci.yml index eee543f74..745ac5e0f 100644 --- a/.github/workflows/update-docker-ci.yml +++ b/.github/workflows/update-docker-ci.yml @@ -60,7 +60,7 @@ jobs: - name: Get changed files id: changed-files - uses: tj-actions/changed-files@24d32ffd492484c1d75e0c0b894501ddb9d30d62 # v47.0.0 + uses: tj-actions/changed-files@e0021407031f5be11a464abee9a0776171c79891 # v47.0.1 with: files: "docker/compilers/gcc/**" @@ -98,7 +98,7 @@ jobs: - name: Get changed files id: changed-files - uses: tj-actions/changed-files@ed68ef82c095e0d48ec87eccea555d944a631a4c # v46.0.5 + uses: tj-actions/changed-files@e0021407031f5be11a464abee9a0776171c79891 # v47.0.1 with: files: "docker/compilers/gcc/**" @@ -136,7 +136,7 @@ jobs: - name: Get changed files id: changed-files - uses: tj-actions/changed-files@24d32ffd492484c1d75e0c0b894501ddb9d30d62 # v47.0.0 + uses: tj-actions/changed-files@e0021407031f5be11a464abee9a0776171c79891 # v47.0.1 with: files: "docker/compilers/gcc/**" @@ -187,7 +187,7 @@ jobs: - name: Get changed files id: changed-files - uses: tj-actions/changed-files@24d32ffd492484c1d75e0c0b894501ddb9d30d62 # v47.0.0 + uses: tj-actions/changed-files@e0021407031f5be11a464abee9a0776171c79891 # v47.0.1 with: files: "docker/compilers/clang/**" @@ -223,7 +223,7 @@ jobs: - name: Get changed files id: changed-files - uses: tj-actions/changed-files@24d32ffd492484c1d75e0c0b894501ddb9d30d62 # v47.0.0 + uses: tj-actions/changed-files@e0021407031f5be11a464abee9a0776171c79891 # v47.0.1 with: files: "docker/tools/**" @@ -254,7 +254,7 @@ jobs: - name: Get changed files id: changed-files - uses: tj-actions/changed-files@ed68ef82c095e0d48ec87eccea555d944a631a4c # v46.0.5 + uses: tj-actions/changed-files@e0021407031f5be11a464abee9a0776171c79891 # v47.0.1 with: files: "docker/tools/**" @@ -285,7 +285,7 @@ jobs: - name: Get changed files id: changed-files - uses: tj-actions/changed-files@24d32ffd492484c1d75e0c0b894501ddb9d30d62 # v47.0.0 + uses: tj-actions/changed-files@e0021407031f5be11a464abee9a0776171c79891 # v47.0.1 with: files: "docker/tools/**" From b1a49fdaab47f21c606f939fd16f704f03da4106 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Dec 2025 12:35:11 +0000 Subject: [PATCH 06/15] ci: [DEPENDABOT] Bump peter-evans/create-pull-request from 7.0.11 to 8.0.0 (#2854) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/clang-tidy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index 565d25221..0cfd0ea08 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -107,7 +107,7 @@ jobs: - name: Create PR with fixes if: ${{ steps.files_changed.outcome != 'success' && github.event_name != 'pull_request' }} - uses: peter-evans/create-pull-request@22a9089034f40e5a961c8808d113e2c98fb63676 # v7.0.11 + uses: peter-evans/create-pull-request@98357b18bf14b5342f975ff684046ec3b2a07725 # v8.0.0 env: GH_REPO: ${{ github.repository }} GH_TOKEN: ${{ github.token }} From 5a5a79fe30c14ba629c7f76b8aff884327b868ba Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Dec 2025 12:35:19 +0000 Subject: [PATCH 07/15] ci: [DEPENDABOT] Bump actions/download-artifact from 6.0.0 to 7.0.0 (#2855) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/build-clio-docker-image.yml | 2 +- .github/workflows/build.yml | 2 +- .github/workflows/check-libxrpl.yml | 2 +- .github/workflows/reusable-release.yml | 4 ++-- .github/workflows/reusable-test.yml | 4 ++-- .github/workflows/reusable-upload-coverage-report.yml | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build-clio-docker-image.yml b/.github/workflows/build-clio-docker-image.yml index 0427b7663..d73c6bec9 100644 --- a/.github/workflows/build-clio-docker-image.yml +++ b/.github/workflows/build-clio-docker-image.yml @@ -52,7 +52,7 @@ jobs: - name: Download Clio binary from artifact if: ${{ inputs.artifact_name != null }} - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: ${{ inputs.artifact_name }} path: ./docker/clio/artifact/ diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7c86f44d7..10e64db0c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -102,7 +102,7 @@ jobs: steps: - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: clio_server_Linux_Release_gcc diff --git a/.github/workflows/check-libxrpl.yml b/.github/workflows/check-libxrpl.yml index 51b389e4c..e14c3b3df 100644 --- a/.github/workflows/check-libxrpl.yml +++ b/.github/workflows/check-libxrpl.yml @@ -72,7 +72,7 @@ jobs: image: ghcr.io/xrplf/clio-ci:067449c3f8ae6755ea84752ea2962b589fe56c8f steps: - - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: clio_tests_check_libxrpl diff --git a/.github/workflows/reusable-release.yml b/.github/workflows/reusable-release.yml index 9f068275b..6cfbfeafa 100644 --- a/.github/workflows/reusable-release.yml +++ b/.github/workflows/reusable-release.yml @@ -64,7 +64,7 @@ jobs: with: disable_ccache: true - - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: path: release_artifacts pattern: clio_server_* @@ -72,7 +72,7 @@ jobs: - name: Prepare release artifacts run: .github/scripts/prepare-release-artifacts.sh release_artifacts - - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: path: release_artifacts pattern: clio_deb_package_* diff --git a/.github/workflows/reusable-test.yml b/.github/workflows/reusable-test.yml index 757c9e8f8..5bfdffe64 100644 --- a/.github/workflows/reusable-test.yml +++ b/.github/workflows/reusable-test.yml @@ -58,7 +58,7 @@ jobs: with: fetch-depth: 0 - - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: clio_tests_${{ runner.os }}_${{ inputs.build_type }}_${{ inputs.conan_profile }} @@ -146,7 +146,7 @@ jobs: sleep 5 done - - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + - uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: clio_integration_tests_${{ runner.os }}_${{ inputs.build_type }}_${{ inputs.conan_profile }} diff --git a/.github/workflows/reusable-upload-coverage-report.yml b/.github/workflows/reusable-upload-coverage-report.yml index a223c2744..f0261d0dc 100644 --- a/.github/workflows/reusable-upload-coverage-report.yml +++ b/.github/workflows/reusable-upload-coverage-report.yml @@ -21,7 +21,7 @@ jobs: fetch-depth: 0 - name: Download report artifact - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: coverage-report.xml path: build From 612434677a7a25814808761acb6e56a39cbc52be Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Dec 2025 12:35:49 +0000 Subject: [PATCH 08/15] ci: [DEPENDABOT] Bump actions/upload-artifact from 5.0.0 to 6.0.0 (#2856) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/check-libxrpl.yml | 2 +- .github/workflows/reusable-build.yml | 10 +++++----- .github/workflows/reusable-release.yml | 2 +- .github/workflows/reusable-test.yml | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/check-libxrpl.yml b/.github/workflows/check-libxrpl.yml index e14c3b3df..9a25b81f4 100644 --- a/.github/workflows/check-libxrpl.yml +++ b/.github/workflows/check-libxrpl.yml @@ -59,7 +59,7 @@ jobs: run: strip build/clio_tests - name: Upload clio_tests - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: clio_tests_check_libxrpl path: build/clio_tests diff --git a/.github/workflows/reusable-build.yml b/.github/workflows/reusable-build.yml index e4c9f188c..1acc1e484 100644 --- a/.github/workflows/reusable-build.yml +++ b/.github/workflows/reusable-build.yml @@ -154,7 +154,7 @@ jobs: - name: Upload build time analyze report if: ${{ inputs.analyze_build_time }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: build_time_report_${{ runner.os }}_${{ inputs.build_type }}_${{ inputs.conan_profile }} path: build_time_report.txt @@ -182,28 +182,28 @@ jobs: - name: Upload clio_server if: ${{ inputs.upload_clio_server && !inputs.code_coverage && !inputs.analyze_build_time }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: clio_server_${{ runner.os }}_${{ inputs.build_type }}_${{ inputs.conan_profile }} path: build/clio_server - name: Upload clio_tests if: ${{ !inputs.code_coverage && !inputs.analyze_build_time && !inputs.package }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: clio_tests_${{ runner.os }}_${{ inputs.build_type }}_${{ inputs.conan_profile }} path: build/clio_tests - name: Upload clio_integration_tests if: ${{ !inputs.code_coverage && !inputs.analyze_build_time && !inputs.package }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: clio_integration_tests_${{ runner.os }}_${{ inputs.build_type }}_${{ inputs.conan_profile }} path: build/clio_integration_tests - name: Upload Clio Linux package if: ${{ inputs.package }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: clio_deb_package_${{ runner.os }}_${{ inputs.build_type }}_${{ inputs.conan_profile }} path: build/*.deb diff --git a/.github/workflows/reusable-release.yml b/.github/workflows/reusable-release.yml index 6cfbfeafa..b43020bde 100644 --- a/.github/workflows/reusable-release.yml +++ b/.github/workflows/reusable-release.yml @@ -95,7 +95,7 @@ jobs: cat CHANGELOG.md >> "${RUNNER_TEMP}/release_notes.md" - name: Upload release notes - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: release_notes_${{ inputs.version }} path: "${RUNNER_TEMP}/release_notes.md" diff --git a/.github/workflows/reusable-test.yml b/.github/workflows/reusable-test.yml index 5bfdffe64..273335664 100644 --- a/.github/workflows/reusable-test.yml +++ b/.github/workflows/reusable-test.yml @@ -85,7 +85,7 @@ jobs: - name: Upload sanitizer report if: ${{ env.SANITIZER_IGNORE_ERRORS == 'true' && steps.check_report.outputs.found_report == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: sanitizer_report_${{ runner.os }}_${{ inputs.build_type }}_${{ inputs.conan_profile }} path: .sanitizer-report/* From d5b0329e7066c52ca6e4912c0bed717aee7c22d9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Dec 2025 12:35:58 +0000 Subject: [PATCH 09/15] ci: [DEPENDABOT] Bump codecov/codecov-action from 5.5.1 to 5.5.2 (#2857) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/reusable-upload-coverage-report.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reusable-upload-coverage-report.yml b/.github/workflows/reusable-upload-coverage-report.yml index f0261d0dc..61befa38b 100644 --- a/.github/workflows/reusable-upload-coverage-report.yml +++ b/.github/workflows/reusable-upload-coverage-report.yml @@ -28,7 +28,7 @@ jobs: - name: Upload coverage report if: ${{ hashFiles('build/coverage_report.xml') != '' }} - uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 # v5.5.1 + uses: codecov/codecov-action@671740ac38dd9b0130fbe1cec585b89eea48d3de # v5.5.2 with: files: build/coverage_report.xml fail_ci_if_error: true From db9a460867be48b2b4631a3f21979b375cdeeb34 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 15 Dec 2025 12:36:09 +0000 Subject: [PATCH 10/15] ci: [DEPENDABOT] Bump actions/upload-artifact from 5.0.0 to 6.0.0 in /.github/actions/code-coverage (#2858) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/actions/code-coverage/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/code-coverage/action.yml b/.github/actions/code-coverage/action.yml index 6a89a0a24..fa9f67a4e 100644 --- a/.github/actions/code-coverage/action.yml +++ b/.github/actions/code-coverage/action.yml @@ -24,7 +24,7 @@ runs: -j8 --exclude-throw-branches - name: Archive coverage report - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: coverage-report.xml path: build/coverage_report.xml From 7600e740a008f2d7699cefed80340af5fd24015a Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Tue, 16 Dec 2025 15:06:35 +0000 Subject: [PATCH 11/15] revert: "refactor: Add writing command to etl::SystemState" (#2860) --- src/etl/ETLService.cpp | 20 +++---- src/etl/ETLService.hpp | 2 +- src/etl/SystemState.hpp | 23 +------- src/etl/impl/LedgerPublisher.hpp | 17 +----- src/etl/impl/Loading.cpp | 2 +- tests/unit/etl/ETLServiceTests.cpp | 78 ++----------------------- tests/unit/etl/LedgerPublisherTests.cpp | 4 +- tests/unit/etl/LoadingTests.cpp | 50 ---------------- 8 files changed, 21 insertions(+), 175 deletions(-) diff --git a/src/etl/ETLService.cpp b/src/etl/ETLService.cpp index 595411cb4..6f8c0d4ee 100644 --- a/src/etl/ETLService.cpp +++ b/src/etl/ETLService.cpp @@ -348,21 +348,14 @@ ETLService::startMonitor(uint32_t seq) { monitor_ = monitorProvider_->make(ctx_, backend_, ledgers_, seq); - systemStateWriteCommandSubscription_ = - state_->writeCommandSignal.connect([this](SystemState::WriteCommand command) { - switch (command) { - case etl::SystemState::WriteCommand::StartWriting: - attemptTakeoverWriter(); - break; - case etl::SystemState::WriteCommand::StopWriting: - giveUpWriter(); - break; - } - }); - monitorNewSeqSubscription_ = monitor_->subscribeToNewSequence([this](uint32_t seq) { LOG(log_.info()) << "ETLService (via Monitor) got new seq from db: " << seq; + if (state_->writeConflict) { + LOG(log_.info()) << "Got a write conflict; Giving up writer seat immediately"; + giveUpWriter(); + } + if (not state_->isWriting) { auto const diff = data::synchronousAndRetryOnTimeout([this, seq](auto yield) { return backend_->fetchLedgerDiff(seq, yield); @@ -378,7 +371,7 @@ ETLService::startMonitor(uint32_t seq) monitorDbStalledSubscription_ = monitor_->subscribeToDbStalled([this]() { LOG(log_.warn()) << "ETLService received DbStalled signal from Monitor"; if (not state_->isStrictReadonly and not state_->isWriting) - state_->writeCommandSignal(SystemState::WriteCommand::StartWriting); + attemptTakeoverWriter(); }); monitor_->run(); @@ -411,6 +404,7 @@ ETLService::giveUpWriter() { ASSERT(not state_->isStrictReadonly, "This should only happen on writer nodes"); state_->isWriting = false; + state_->writeConflict = false; taskMan_ = nullptr; } diff --git a/src/etl/ETLService.hpp b/src/etl/ETLService.hpp index 689d4d14d..45185d4be 100644 --- a/src/etl/ETLService.hpp +++ b/src/etl/ETLService.hpp @@ -74,6 +74,7 @@ #include #include #include +#include namespace etl { @@ -116,7 +117,6 @@ class ETLService : public ETLServiceInterface { boost::signals2::scoped_connection monitorNewSeqSubscription_; boost::signals2::scoped_connection monitorDbStalledSubscription_; - boost::signals2::scoped_connection systemStateWriteCommandSubscription_; std::optional> mainLoop_; diff --git a/src/etl/SystemState.hpp b/src/etl/SystemState.hpp index 188d53fcb..7f841665f 100644 --- a/src/etl/SystemState.hpp +++ b/src/etl/SystemState.hpp @@ -23,8 +23,7 @@ #include "util/prometheus/Label.hpp" #include "util/prometheus/Prometheus.hpp" -#include -#include +#include namespace etl { @@ -51,24 +50,8 @@ struct SystemState { "Whether the process is writing to the database" ); - /** - * @brief Commands for controlling the ETL writer state. - * - * These commands are emitted via writeCommandSignal to coordinate writer state transitions across components. - */ - enum class WriteCommand { - StartWriting, /**< Request to attempt taking over as the ETL writer */ - StopWriting /**< Request to give up the ETL writer role (e.g., due to write conflict) */ - }; - - /** - * @brief Signal for coordinating ETL writer state transitions. - * - * This signal allows components to request changes to the writer state without direct coupling. - * - Emitted with StartWriting when database stalls and node should attempt to become writer - * - Emitted with StopWriting when write conflicts are detected - */ - boost::signals2::signal writeCommandSignal; + std::atomic_bool isStopping = false; /**< @brief Whether the software is stopping. */ + std::atomic_bool writeConflict = false; /**< @brief Whether a write conflict was detected. */ /** * @brief Whether clio detected an amendment block. diff --git a/src/etl/impl/LedgerPublisher.hpp b/src/etl/impl/LedgerPublisher.hpp index 1db50299b..0b48ca3f6 100644 --- a/src/etl/impl/LedgerPublisher.hpp +++ b/src/etl/impl/LedgerPublisher.hpp @@ -45,7 +45,6 @@ #include #include -#include #include #include #include @@ -77,8 +76,6 @@ class LedgerPublisher : public LedgerPublisherInterface { util::async::AnyStrand publishStrand_; - std::atomic_bool stop_{false}; - std::shared_ptr backend_; std::shared_ptr subscriptions_; std::reference_wrapper state_; // shared state for ETL @@ -128,7 +125,7 @@ public: { LOG(log_.info()) << "Attempting to publish ledger = " << ledgerSequence; size_t numAttempts = 0; - while (not stop_) { + while (not state_.get().isStopping) { auto range = backend_->hardFetchLedgerRangeNoThrow(); if (!range || range->maxSequence < ledgerSequence) { @@ -261,18 +258,6 @@ public: return *lastPublishedSequence_.lock(); } - /** - * @brief Stops publishing - * - * @note This is a basic implementation to satisfy tests. This will be improved in - * https://github.com/XRPLF/clio/issues/2833 - */ - void - stop() - { - stop_ = true; - } - private: void setLastClose(std::chrono::time_point lastCloseTime) diff --git a/src/etl/impl/Loading.cpp b/src/etl/impl/Loading.cpp index 9c378f253..59d2d0a9c 100644 --- a/src/etl/impl/Loading.cpp +++ b/src/etl/impl/Loading.cpp @@ -75,7 +75,7 @@ Loader::load(model::LedgerData const& data) << "; took " << duration << "ms"; if (not success) { - state_->writeCommandSignal(SystemState::WriteCommand::StopWriting); + state_->writeConflict = true; LOG(log_.warn()) << "Another node wrote a ledger into the DB - we have a write conflict"; return std::unexpected(LoaderError::WriteConflict); } diff --git a/tests/unit/etl/ETLServiceTests.cpp b/tests/unit/etl/ETLServiceTests.cpp index 59a570a44..253009459 100644 --- a/tests/unit/etl/ETLServiceTests.cpp +++ b/tests/unit/etl/ETLServiceTests.cpp @@ -216,10 +216,6 @@ protected: std::shared_ptr> monitorProvider_ = std::make_shared>(); std::shared_ptr systemState_ = std::make_shared(); - testing::StrictMock> mockWriteSignalCommandCallback_; - boost::signals2::scoped_connection writeCommandConnection_{ - systemState_->writeCommandSignal.connect(mockWriteSignalCommandCallback_.AsStdFunction()) - }; etl::ETLService service_{ ctx_, @@ -374,13 +370,13 @@ TEST_F(ETLServiceTests, HandlesWriteConflictInMonitorSubscription) EXPECT_CALL(*cacheLoader_, load(kSEQ)); service_.run(); - writeCommandConnection_.disconnect(); - systemState_->writeCommandSignal(etl::SystemState::WriteCommand::StopWriting); + systemState_->writeConflict = true; EXPECT_CALL(*publisher_, publish(kSEQ + 1, testing::_, testing::_)); ASSERT_TRUE(capturedCallback); capturedCallback(kSEQ + 1); + EXPECT_FALSE(systemState_->writeConflict); EXPECT_FALSE(systemState_->isWriting); } @@ -451,8 +447,6 @@ TEST_F(ETLServiceTests, AttemptTakeoverWriter) EXPECT_CALL(*taskManagerProvider_, make(testing::_, testing::_, kSEQ + 1, testing::_)) .WillOnce(testing::Return(std::move(mockTaskManager))); - EXPECT_CALL(mockWriteSignalCommandCallback_, Call(etl::SystemState::WriteCommand::StartWriting)); - ASSERT_TRUE(capturedDbStalledCallback); capturedDbStalledCallback(); @@ -483,15 +477,15 @@ TEST_F(ETLServiceTests, GiveUpWriterAfterWriteConflict) service_.run(); systemState_->isWriting = true; - writeCommandConnection_.disconnect(); - systemState_->writeCommandSignal(etl::SystemState::WriteCommand::StopWriting); + systemState_->writeConflict = true; // got a write conflict along the way EXPECT_CALL(*publisher_, publish(kSEQ + 1, testing::_, testing::_)); ASSERT_TRUE(capturedCallback); capturedCallback(kSEQ + 1); - EXPECT_FALSE(systemState_->isWriting); // gives up writing + EXPECT_FALSE(systemState_->isWriting); // gives up writing + EXPECT_FALSE(systemState_->writeConflict); // and removes write conflict flag } TEST_F(ETLServiceTests, CancelledLoadInitialLedger) @@ -545,65 +539,3 @@ TEST_F(ETLServiceTests, RunStopsIfInitialLoadIsCancelledByBalancer) EXPECT_FALSE(service_.isAmendmentBlocked()); EXPECT_FALSE(service_.isCorruptionDetected()); } - -TEST_F(ETLServiceTests, DbStalledDoesNotTriggerSignalWhenStrictReadonly) -{ - auto mockMonitor = std::make_unique>(); - auto& mockMonitorRef = *mockMonitor; - std::function capturedDbStalledCallback; - - EXPECT_CALL(*monitorProvider_, make).WillOnce([&mockMonitor](auto, auto, auto, auto, auto) { - return std::move(mockMonitor); - }); - EXPECT_CALL(mockMonitorRef, subscribeToNewSequence); - EXPECT_CALL(mockMonitorRef, subscribeToDbStalled).WillOnce([&capturedDbStalledCallback](auto callback) { - capturedDbStalledCallback = callback; - return boost::signals2::scoped_connection{}; - }); - EXPECT_CALL(mockMonitorRef, run); - - EXPECT_CALL(*backend_, hardFetchLedgerRange) - .WillRepeatedly(testing::Return(data::LedgerRange{.minSequence = 1, .maxSequence = kSEQ})); - EXPECT_CALL(*ledgers_, getMostRecent()).WillOnce(testing::Return(kSEQ)); - EXPECT_CALL(*cacheLoader_, load(kSEQ)); - - service_.run(); - systemState_->isStrictReadonly = true; // strict readonly mode - systemState_->isWriting = false; - - // No signal should be emitted because node is in strict readonly mode - - ASSERT_TRUE(capturedDbStalledCallback); - capturedDbStalledCallback(); -} - -TEST_F(ETLServiceTests, DbStalledDoesNotTriggerSignalWhenAlreadyWriting) -{ - auto mockMonitor = std::make_unique>(); - auto& mockMonitorRef = *mockMonitor; - std::function capturedDbStalledCallback; - - EXPECT_CALL(*monitorProvider_, make).WillOnce([&mockMonitor](auto, auto, auto, auto, auto) { - return std::move(mockMonitor); - }); - EXPECT_CALL(mockMonitorRef, subscribeToNewSequence); - EXPECT_CALL(mockMonitorRef, subscribeToDbStalled).WillOnce([&capturedDbStalledCallback](auto callback) { - capturedDbStalledCallback = callback; - return boost::signals2::scoped_connection{}; - }); - EXPECT_CALL(mockMonitorRef, run); - - EXPECT_CALL(*backend_, hardFetchLedgerRange) - .WillRepeatedly(testing::Return(data::LedgerRange{.minSequence = 1, .maxSequence = kSEQ})); - EXPECT_CALL(*ledgers_, getMostRecent()).WillOnce(testing::Return(kSEQ)); - EXPECT_CALL(*cacheLoader_, load(kSEQ)); - - service_.run(); - systemState_->isStrictReadonly = false; - systemState_->isWriting = true; // already writing - - // No signal should be emitted because node is already writing - - ASSERT_TRUE(capturedDbStalledCallback); - capturedDbStalledCallback(); -} diff --git a/tests/unit/etl/LedgerPublisherTests.cpp b/tests/unit/etl/LedgerPublisherTests.cpp index 5b3c73d0f..e4d422a9d 100644 --- a/tests/unit/etl/LedgerPublisherTests.cpp +++ b/tests/unit/etl/LedgerPublisherTests.cpp @@ -216,14 +216,15 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerHeaderCloseTimeGreaterThanNow) TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqStopIsTrue) { auto dummyState = etl::SystemState{}; + dummyState.isStopping = true; auto publisher = impl::LedgerPublisher(ctx, backend_, mockSubscriptionManagerPtr, dummyState); - publisher.stop(); EXPECT_FALSE(publisher.publish(kSEQ, {})); } TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqMaxAttempt) { auto dummyState = etl::SystemState{}; + dummyState.isStopping = false; auto publisher = impl::LedgerPublisher(ctx, backend_, mockSubscriptionManagerPtr, dummyState); static constexpr auto kMAX_ATTEMPT = 2; @@ -237,6 +238,7 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqMaxAttempt) TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqStopIsFalse) { auto dummyState = etl::SystemState{}; + dummyState.isStopping = false; auto publisher = impl::LedgerPublisher(ctx, backend_, mockSubscriptionManagerPtr, dummyState); LedgerRange const range{.minSequence = kSEQ, .maxSequence = kSEQ}; diff --git a/tests/unit/etl/LoadingTests.cpp b/tests/unit/etl/LoadingTests.cpp index 1554d7d1b..5904027ee 100644 --- a/tests/unit/etl/LoadingTests.cpp +++ b/tests/unit/etl/LoadingTests.cpp @@ -189,53 +189,3 @@ TEST_F(LoadingAssertTest, LoadInitialLedgerHasDataInDB) EXPECT_CLIO_ASSERT_FAIL({ [[maybe_unused]] auto unused = loader_.loadInitialLedger(data); }); } - -TEST_F(LoadingTests, LoadWriteConflictEmitsStopWritingSignal) -{ - state_->isWriting = true; // writer is active - auto const data = createTestData(); - testing::StrictMock> mockSignalCallback; - - auto connection = state_->writeCommandSignal.connect(mockSignalCallback.AsStdFunction()); - - EXPECT_CALL(*mockRegistryPtr_, dispatch(data)); - EXPECT_CALL(*backend_, doFinishWrites()).WillOnce(testing::Return(false)); // simulate write conflict - EXPECT_CALL(mockSignalCallback, Call(etl::SystemState::WriteCommand::StopWriting)); - - auto result = loader_.load(data); - EXPECT_FALSE(result.has_value()); - EXPECT_EQ(result.error(), etl::LoaderError::WriteConflict); -} - -TEST_F(LoadingTests, LoadSuccessDoesNotEmitSignal) -{ - state_->isWriting = true; // writer is active - auto const data = createTestData(); - testing::StrictMock> mockSignalCallback; - - auto connection = state_->writeCommandSignal.connect(mockSignalCallback.AsStdFunction()); - - EXPECT_CALL(*mockRegistryPtr_, dispatch(data)); - EXPECT_CALL(*backend_, doFinishWrites()).WillOnce(testing::Return(true)); // success - // No signal should be emitted on success - - auto result = loader_.load(data); - EXPECT_TRUE(result.has_value()); -} - -TEST_F(LoadingTests, LoadWhenNotWritingDoesNotCheckConflict) -{ - state_->isWriting = false; // not a writer - auto const data = createTestData(); - testing::StrictMock> mockSignalCallback; - - auto connection = state_->writeCommandSignal.connect(mockSignalCallback.AsStdFunction()); - - EXPECT_CALL(*mockRegistryPtr_, dispatch(data)); - // doFinishWrites should not be called when not writing - EXPECT_CALL(*backend_, doFinishWrites()).Times(0); - // No signal should be emitted - - auto result = loader_.load(data); - EXPECT_TRUE(result.has_value()); -} From 4b731a92ae24888fa2ad670511e8d6db5ddd1ef4 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Tue, 16 Dec 2025 20:54:48 +0000 Subject: [PATCH 12/15] ci: Update shared actions (#2852) --- .github/actions/build-clio/action.yml | 2 +- .github/workflows/check-libxrpl.yml | 2 +- .github/workflows/clang-tidy.yml | 4 ++-- .github/workflows/docs.yml | 2 +- .github/workflows/reusable-build.yml | 4 ++-- .github/workflows/reusable-release.yml | 2 +- .github/workflows/reusable-test.yml | 4 ++-- .github/workflows/upload-conan-deps.yml | 2 +- 8 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/actions/build-clio/action.yml b/.github/actions/build-clio/action.yml index 86f7ff05e..6fa94eef2 100644 --- a/.github/actions/build-clio/action.yml +++ b/.github/actions/build-clio/action.yml @@ -14,7 +14,7 @@ runs: using: composite steps: - name: Get number of processors - uses: XRPLF/actions/.github/actions/get-nproc@046b1620f6bfd6cd0985dc82c3df02786801fe0a + uses: XRPLF/actions/get-nproc@cf0433aa74563aead044a1e395610c96d65a37cf id: nproc with: subtract: ${{ inputs.nproc_subtract }} diff --git a/.github/workflows/check-libxrpl.yml b/.github/workflows/check-libxrpl.yml index 9a25b81f4..4e5b744ae 100644 --- a/.github/workflows/check-libxrpl.yml +++ b/.github/workflows/check-libxrpl.yml @@ -29,7 +29,7 @@ jobs: fetch-depth: 0 - name: Prepare runner - uses: XRPLF/actions/.github/actions/prepare-runner@8abb0722cbff83a9a2dc7d06c473f7a4964b7382 + uses: XRPLF/actions/prepare-runner@2ece4ec6ab7de266859a6f053571425b2bd684b6 with: disable_ccache: true diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml index 0cfd0ea08..db75ab300 100644 --- a/.github/workflows/clang-tidy.yml +++ b/.github/workflows/clang-tidy.yml @@ -44,7 +44,7 @@ jobs: fetch-depth: 0 - name: Prepare runner - uses: XRPLF/actions/.github/actions/prepare-runner@8abb0722cbff83a9a2dc7d06c473f7a4964b7382 + uses: XRPLF/actions/prepare-runner@2ece4ec6ab7de266859a6f053571425b2bd684b6 with: disable_ccache: true @@ -59,7 +59,7 @@ jobs: conan_profile: ${{ env.CONAN_PROFILE }} - name: Get number of processors - uses: XRPLF/actions/.github/actions/get-nproc@046b1620f6bfd6cd0985dc82c3df02786801fe0a + uses: XRPLF/actions/get-nproc@cf0433aa74563aead044a1e395610c96d65a37cf id: nproc - name: Run clang-tidy (several times) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index c3347330c..f35b872bd 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -27,7 +27,7 @@ jobs: lfs: true - name: Prepare runner - uses: XRPLF/actions/.github/actions/prepare-runner@8abb0722cbff83a9a2dc7d06c473f7a4964b7382 + uses: XRPLF/actions/prepare-runner@2ece4ec6ab7de266859a6f053571425b2bd684b6 with: disable_ccache: true diff --git a/.github/workflows/reusable-build.yml b/.github/workflows/reusable-build.yml index 1acc1e484..4586097e8 100644 --- a/.github/workflows/reusable-build.yml +++ b/.github/workflows/reusable-build.yml @@ -88,7 +88,7 @@ jobs: steps: - name: Cleanup workspace if: ${{ runner.os == 'macOS' }} - uses: XRPLF/actions/.github/actions/cleanup-workspace@ea9970b7c211b18f4c8bcdb28c29f5711752029f + uses: XRPLF/actions/cleanup-workspace@cf0433aa74563aead044a1e395610c96d65a37cf - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: @@ -99,7 +99,7 @@ jobs: ref: ${{ github.ref }} - name: Prepare runner - uses: XRPLF/actions/.github/actions/prepare-runner@8abb0722cbff83a9a2dc7d06c473f7a4964b7382 + uses: XRPLF/actions/prepare-runner@2ece4ec6ab7de266859a6f053571425b2bd684b6 with: disable_ccache: ${{ !inputs.download_ccache }} diff --git a/.github/workflows/reusable-release.yml b/.github/workflows/reusable-release.yml index b43020bde..c29e26518 100644 --- a/.github/workflows/reusable-release.yml +++ b/.github/workflows/reusable-release.yml @@ -60,7 +60,7 @@ jobs: fetch-depth: 0 - name: Prepare runner - uses: XRPLF/actions/.github/actions/prepare-runner@8abb0722cbff83a9a2dc7d06c473f7a4964b7382 + uses: XRPLF/actions/prepare-runner@2ece4ec6ab7de266859a6f053571425b2bd684b6 with: disable_ccache: true diff --git a/.github/workflows/reusable-test.yml b/.github/workflows/reusable-test.yml index 273335664..f7ca51926 100644 --- a/.github/workflows/reusable-test.yml +++ b/.github/workflows/reusable-test.yml @@ -52,7 +52,7 @@ jobs: steps: - name: Cleanup workspace if: ${{ runner.os == 'macOS' }} - uses: XRPLF/actions/.github/actions/cleanup-workspace@ea9970b7c211b18f4c8bcdb28c29f5711752029f + uses: XRPLF/actions/cleanup-workspace@cf0433aa74563aead044a1e395610c96d65a37cf - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: @@ -124,7 +124,7 @@ jobs: steps: - name: Cleanup workspace if: ${{ runner.os == 'macOS' }} - uses: XRPLF/actions/.github/actions/cleanup-workspace@ea9970b7c211b18f4c8bcdb28c29f5711752029f + uses: XRPLF/actions/cleanup-workspace@cf0433aa74563aead044a1e395610c96d65a37cf - name: Spin up scylladb if: ${{ runner.os == 'macOS' }} diff --git a/.github/workflows/upload-conan-deps.yml b/.github/workflows/upload-conan-deps.yml index dd31d3bc5..07e05751e 100644 --- a/.github/workflows/upload-conan-deps.yml +++ b/.github/workflows/upload-conan-deps.yml @@ -78,7 +78,7 @@ jobs: - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 - name: Prepare runner - uses: XRPLF/actions/.github/actions/prepare-runner@8abb0722cbff83a9a2dc7d06c473f7a4964b7382 + uses: XRPLF/actions/prepare-runner@2ece4ec6ab7de266859a6f053571425b2bd684b6 with: disable_ccache: true From 89fbcbf66a2bb06ac53e4cd151649755e6183a36 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 17 Dec 2025 10:19:20 +0000 Subject: [PATCH 13/15] style: clang-tidy auto fixes (#2862) --- tests/unit/etl/LoadingTests.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/etl/LoadingTests.cpp b/tests/unit/etl/LoadingTests.cpp index 5904027ee..143f915a2 100644 --- a/tests/unit/etl/LoadingTests.cpp +++ b/tests/unit/etl/LoadingTests.cpp @@ -19,7 +19,6 @@ #include "data/Types.hpp" #include "etl/InitialLoadObserverInterface.hpp" -#include "etl/LoaderInterface.hpp" #include "etl/Models.hpp" #include "etl/RegistryInterface.hpp" #include "etl/SystemState.hpp" From 5269ea022334b472d4a6376a5fe989ad1c41b96c Mon Sep 17 00:00:00 2001 From: Bart Date: Thu, 18 Dec 2025 10:15:00 -0500 Subject: [PATCH 14/15] ci: Remove unnecessary creation of build directory (#2867) --- .github/actions/conan/action.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/actions/conan/action.yml b/.github/actions/conan/action.yml index 5c6a0ddd6..897c3e548 100644 --- a/.github/actions/conan/action.yml +++ b/.github/actions/conan/action.yml @@ -21,10 +21,6 @@ inputs: runs: using: composite steps: - - name: Create build directory - shell: bash - run: mkdir -p "${{ inputs.build_dir }}" - - name: Run conan shell: bash env: From 2327e81b0ba2cb9808c8690aa09c0d5ba8200c22 Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Fri, 19 Dec 2025 15:26:55 +0000 Subject: [PATCH 15/15] fix: WorkQueue contention (#2866) Co-authored-by: Ayaz Salikhov --- benchmarks/CMakeLists.txt | 4 +- benchmarks/rpc/WorkQueueBenchmarks.cpp | 127 +++++++++++++++++++++++++ src/rpc/WorkQueue.cpp | 48 ++++------ src/rpc/WorkQueue.hpp | 23 +++++ 4 files changed, 171 insertions(+), 31 deletions(-) create mode 100644 benchmarks/rpc/WorkQueueBenchmarks.cpp diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index ec2d91eb9..e70ee2bb4 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -9,10 +9,12 @@ target_sources( util/async/ExecutionContextBenchmarks.cpp # Logger util/log/LoggerBenchmark.cpp + # WorkQueue + rpc/WorkQueueBenchmarks.cpp ) include(deps/gbench) target_include_directories(clio_benchmark PRIVATE .) -target_link_libraries(clio_benchmark PUBLIC clio_util benchmark::benchmark_main spdlog::spdlog) +target_link_libraries(clio_benchmark PUBLIC clio_util clio_rpc benchmark::benchmark_main spdlog::spdlog) set_target_properties(clio_benchmark PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}) diff --git a/benchmarks/rpc/WorkQueueBenchmarks.cpp b/benchmarks/rpc/WorkQueueBenchmarks.cpp new file mode 100644 index 000000000..9b9ce6947 --- /dev/null +++ b/benchmarks/rpc/WorkQueueBenchmarks.cpp @@ -0,0 +1,127 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2025, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include "rpc/WorkQueue.hpp" +#include "util/Assert.hpp" +#include "util/config/Array.hpp" +#include "util/config/ConfigConstraints.hpp" +#include "util/config/ConfigDefinition.hpp" +#include "util/config/ConfigValue.hpp" +#include "util/config/Types.hpp" +#include "util/log/Logger.hpp" +#include "util/prometheus/Prometheus.hpp" + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +using namespace rpc; +using namespace util::config; + +namespace { + +auto const kCONFIG = ClioConfigDefinition{ + {"prometheus.compress_reply", ConfigValue{ConfigType::Boolean}.defaultValue(true)}, + {"prometheus.enabled", ConfigValue{ConfigType::Boolean}.defaultValue(true)}, + {"log.channels.[].channel", Array{ConfigValue{ConfigType::String}}}, + {"log.channels.[].level", Array{ConfigValue{ConfigType::String}}}, + {"log.level", ConfigValue{ConfigType::String}.defaultValue("info")}, + {"log.format", ConfigValue{ConfigType::String}.defaultValue(R"(%Y-%m-%d %H:%M:%S.%f %^%3!l:%n%$ - %v)")}, + {"log.is_async", ConfigValue{ConfigType::Boolean}.defaultValue(false)}, + {"log.enable_console", ConfigValue{ConfigType::Boolean}.defaultValue(false)}, + {"log.directory", ConfigValue{ConfigType::String}.optional()}, + {"log.rotation_size", ConfigValue{ConfigType::Integer}.defaultValue(2048).withConstraint(gValidateUint32)}, + {"log.directory_max_files", ConfigValue{ConfigType::Integer}.defaultValue(25).withConstraint(gValidateUint32)}, + {"log.tag_style", ConfigValue{ConfigType::String}.defaultValue("none")}, +}; + +// this should be a fixture but it did not work with Args very well +void +init() +{ + static std::once_flag kONCE; + std::call_once(kONCE, [] { + PrometheusService::init(kCONFIG); + (void)util::LogService::init(kCONFIG); + }); +} + +} // namespace + +static void +benchmarkWorkQueue(benchmark::State& state) +{ + init(); + + auto const total = static_cast(state.range(0)); + auto const numThreads = static_cast(state.range(1)); + auto const maxSize = static_cast(state.range(2)); + auto const delayMs = static_cast(state.range(3)); + + for (auto _ : state) { + std::atomic_size_t totalExecuted = 0uz; + std::atomic_size_t totalQueued = 0uz; + + state.PauseTiming(); + WorkQueue queue(numThreads, maxSize); + state.ResumeTiming(); + + for (auto i = 0uz; i < total; ++i) { + totalQueued += static_cast(queue.postCoro( + [&delayMs, &totalExecuted](auto yield) { + ++totalExecuted; + + boost::asio::steady_timer timer(yield.get_executor(), std::chrono::milliseconds{delayMs}); + timer.async_wait(yield); + }, + /* isWhiteListed = */ false + )); + } + + queue.stop(); + + ASSERT(totalExecuted == totalQueued, "Totals don't match"); + ASSERT(totalQueued <= total, "Queued more than requested"); + ASSERT(totalQueued >= maxSize, "Queued less than maxSize"); + } +} + +// Usage example: +/* + ./clio_benchmark \ + --benchmark_repetitions=10 \ + --benchmark_display_aggregates_only=true \ + --benchmark_min_time=1x \ + --benchmark_filter="WorkQueue" +*/ +// TODO: figure out what happens on 1 thread +BENCHMARK(benchmarkWorkQueue) + ->ArgsProduct({{1'000, 10'000, 100'000}, {2, 4, 8}, {0, 5'000}, {10, 100, 250}}) + ->Unit(benchmark::kMillisecond); diff --git a/src/rpc/WorkQueue.cpp b/src/rpc/WorkQueue.cpp index 34617dec3..b676fc64b 100644 --- a/src/rpc/WorkQueue.cpp +++ b/src/rpc/WorkQueue.cpp @@ -34,8 +34,8 @@ #include #include #include +#include #include -#include namespace rpc { @@ -122,7 +122,7 @@ WorkQueue::dispatcherLoop(boost::asio::yield_context yield) // all ongoing tasks must be completed before stopping fully while (not stopping_ or size() > 0) { - std::vector batch; + std::optional task; { auto state = dispatcherState_.lock(); @@ -130,43 +130,31 @@ WorkQueue::dispatcherLoop(boost::asio::yield_context yield) if (state->empty()) { state->isIdle = true; } else { - for (auto count = 0uz; count < kTAKE_HIGH_PRIO and not state->high.empty(); ++count) { - batch.push_back(std::move(state->high.front())); - state->high.pop(); - } - - if (not state->normal.empty()) { - batch.push_back(std::move(state->normal.front())); - state->normal.pop(); - } + task = state->popNext(); } } - if (not stopping_ and batch.empty()) { + if (not stopping_ and not task.has_value()) { waitTimer_.expires_at(std::chrono::steady_clock::time_point::max()); boost::system::error_code ec; waitTimer_.async_wait(yield[ec]); - } else { - for (auto task : std::move(batch)) { - util::spawn( - ioc_, - [this, spawnedAt = std::chrono::system_clock::now(), task = std::move(task)](auto yield) mutable { - auto const takenAt = std::chrono::system_clock::now(); - auto const waited = - std::chrono::duration_cast(takenAt - spawnedAt).count(); + } else if (task.has_value()) { + util::spawn( + ioc_, + [this, spawnedAt = std::chrono::system_clock::now(), task = std::move(*task)](auto yield) mutable { + auto const takenAt = std::chrono::system_clock::now(); + auto const waited = + std::chrono::duration_cast(takenAt - spawnedAt).count(); - ++queued_.get(); - durationUs_.get() += waited; - LOG(log_.info()) << "WorkQueue wait time: " << waited << ", queue size: " << size(); + ++queued_.get(); + durationUs_.get() += waited; + LOG(log_.info()) << "WorkQueue wait time: " << waited << ", queue size: " << size(); - task(yield); + task(yield); - --curSize_.get(); - } - ); - } - - boost::asio::post(ioc_.get_executor(), yield); // yield back to avoid hijacking the thread + --curSize_.get(); + } + ); } } diff --git a/src/rpc/WorkQueue.hpp b/src/rpc/WorkQueue.hpp index 702a173df..8fa466501 100644 --- a/src/rpc/WorkQueue.hpp +++ b/src/rpc/WorkQueue.hpp @@ -38,7 +38,9 @@ #include #include #include +#include #include +#include namespace rpc { @@ -79,6 +81,7 @@ private: QueueType normal; bool isIdle = false; + size_t highPriorityCounter = 0; void push(Priority priority, auto&& task) @@ -96,6 +99,26 @@ private: { return high.empty() and normal.empty(); } + + [[nodiscard]] std::optional + popNext() + { + if (not high.empty() and (highPriorityCounter < kTAKE_HIGH_PRIO or normal.empty())) { + auto task = std::move(high.front()); + high.pop(); + ++highPriorityCounter; + return task; + } + + if (not normal.empty()) { + auto task = std::move(normal.front()); + normal.pop(); + highPriorityCounter = 0; + return task; + } + + return std::nullopt; + } }; private: