From c7fee023e77f1b7aa502771a47bd0083d2498ee6 Mon Sep 17 00:00:00 2001
From: cyan317 <120398799+cindyyan317@users.noreply.github.com>
Date: Mon, 24 Jun 2024 13:05:07 +0100
Subject: [PATCH] Fix issue: "Updating cache" prints in log when cache is
 disabled (#1479)

Fixed #1461
---
 src/data/BackendInterface.cpp           |  1 -
 src/data/CassandraBackend.hpp           |  4 ++--
 src/etl/impl/LedgerPublisher.hpp        | 13 ++++++++-----
 tests/unit/etl/LedgerPublisherTests.cpp | 26 ++++++++++++++++++++++---
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/src/data/BackendInterface.cpp b/src/data/BackendInterface.cpp
index 4db752e6c..00b7ce397 100644
--- a/src/data/BackendInterface.cpp
+++ b/src/data/BackendInterface.cpp
@@ -93,7 +93,6 @@ BackendInterface::fetchLedgerObject(
         return obj;
     }
 
-    LOG(gLog.trace()) << "Cache miss - " << ripple::strHex(key);
     auto dbObj = doFetchLedgerObject(key, sequence, yield);
     if (!dbObj) {
         LOG(gLog.trace()) << "Missed cache and missed in db";
diff --git a/src/data/CassandraBackend.hpp b/src/data/CassandraBackend.hpp
index 24e2ae670..bedda2698 100644
--- a/src/data/CassandraBackend.hpp
+++ b/src/data/CassandraBackend.hpp
@@ -336,8 +336,8 @@ class BasicCassandraBackend : public BackendInterface {
 
         auto const& result = res.value();
         if (not result.hasRows()) {
-            LOG(log_.error()) << "Could not fetch all transaction hashes - no rows; ledger = "
-                              << std::to_string(ledgerSequence);
+            LOG(log_.warn()) << "Could not fetch all transaction hashes - no rows; ledger = "
+                             << std::to_string(ledgerSequence);
             return {};
         }
 
diff --git a/src/etl/impl/LedgerPublisher.hpp b/src/etl/impl/LedgerPublisher.hpp
index 9acfac6f2..d03f92a48 100644
--- a/src/etl/impl/LedgerPublisher.hpp
+++ b/src/etl/impl/LedgerPublisher.hpp
@@ -166,13 +166,16 @@ class LedgerPublisher {
             LOG(log_.info()) << "Publishing ledger " << std::to_string(lgrInfo.seq);
 
             if (!state_.get().isWriting) {
-                LOG(log_.info()) << "Updating cache";
+                LOG(log_.info()) << "Updating ledger range for read node.";
 
-                std::vector<data::LedgerObject> const diff = data::synchronousAndRetryOnTimeout([&](auto yield) {
-                    return backend_->fetchLedgerDiff(lgrInfo.seq, yield);
-                });
+                if (!cache_.get().isDisabled()) {
+                    std::vector<data::LedgerObject> const diff = data::synchronousAndRetryOnTimeout([&](auto yield) {
+                        return backend_->fetchLedgerDiff(lgrInfo.seq, yield);
+                    });
+
+                    cache_.get().update(diff, lgrInfo.seq);
+                }
 
-                cache_.get().update(diff, lgrInfo.seq);
                 backend_->updateRange(lgrInfo.seq);
             }
 
diff --git a/tests/unit/etl/LedgerPublisherTests.cpp b/tests/unit/etl/LedgerPublisherTests.cpp
index 0d2b780b3..9a90a2d81 100644
--- a/tests/unit/etl/LedgerPublisherTests.cpp
+++ b/tests/unit/etl/LedgerPublisherTests.cpp
@@ -68,15 +68,35 @@ struct ETLLedgerPublisherTest : util::prometheus::WithPrometheus, MockBackendTes
     StrictMockSubscriptionManagerSharedPtr mockSubscriptionManagerPtr;
 };
 
-TEST_F(ETLLedgerPublisherTest, PublishLedgerHeaderIsWritingFalse)
+TEST_F(ETLLedgerPublisherTest, PublishLedgerHeaderIsWritingFalseAndCacheDisabled)
 {
     SystemState dummyState;
     dummyState.isWriting = false;
     auto const dummyLedgerHeader = CreateLedgerHeader(LEDGERHASH, SEQ, AGE);
     impl::LedgerPublisher publisher(ctx, backend, mockCache, mockSubscriptionManagerPtr, dummyState);
     publisher.publish(dummyLedgerHeader);
+    EXPECT_CALL(mockCache, isDisabled).WillOnce(Return(true));
+    EXPECT_CALL(*backend, fetchLedgerDiff(SEQ, _)).Times(0);
 
-    EXPECT_CALL(*backend, fetchLedgerDiff(SEQ, _)).WillOnce(Return(std::vector<LedgerObject>{}));
+    // setLastPublishedSequence not in strand, should verify before run
+    EXPECT_TRUE(publisher.getLastPublishedSequence());
+    EXPECT_EQ(publisher.getLastPublishedSequence().value(), SEQ);
+
+    ctx.run();
+    EXPECT_TRUE(backend->fetchLedgerRange());
+    EXPECT_EQ(backend->fetchLedgerRange().value().minSequence, SEQ);
+    EXPECT_EQ(backend->fetchLedgerRange().value().maxSequence, SEQ);
+}
+
+TEST_F(ETLLedgerPublisherTest, PublishLedgerHeaderIsWritingFalseAndCacheEnabled)
+{
+    SystemState dummyState;
+    dummyState.isWriting = false;
+    auto const dummyLedgerHeader = CreateLedgerHeader(LEDGERHASH, SEQ, AGE);
+    impl::LedgerPublisher publisher(ctx, backend, mockCache, mockSubscriptionManagerPtr, dummyState);
+    publisher.publish(dummyLedgerHeader);
+    EXPECT_CALL(mockCache, isDisabled).WillOnce(Return(false));
+    EXPECT_CALL(*backend, fetchLedgerDiff(SEQ, _)).Times(1);
 
     // setLastPublishedSequence not in strand, should verify before run
     EXPECT_TRUE(publisher.getLastPublishedSequence());
@@ -218,7 +238,7 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqStopIsFalse)
 
     auto const dummyLedgerHeader = CreateLedgerHeader(LEDGERHASH, SEQ, AGE);
     EXPECT_CALL(*backend, fetchLedgerBySequence(SEQ, _)).WillOnce(Return(dummyLedgerHeader));
-
+    EXPECT_CALL(mockCache, isDisabled).WillOnce(Return(false));
     EXPECT_CALL(*backend, fetchLedgerDiff(SEQ, _)).WillOnce(Return(std::vector<LedgerObject>{}));
     EXPECT_CALL(mockCache, updateImp);