From 2093c882298e0dc363fc56727b8e1c4a14d0c2f2 Mon Sep 17 00:00:00 2001
From: Yin Congmin <congmin.yin@intel.com>
Date: Sat, 26 Dec 2020 00:01:34 +0800
Subject: [PATCH] tools: update immutable object cache water mark

Signed-off-by: Yin Congmin <congmin.yin@intel.com>
---
 PendingReleaseNotes                                  |  4 ++++
 doc/rbd/rbd-persistent-read-only-cache.rst           |  6 +++---
 src/common/options.cc                                |  2 +-
 src/test/immutable_object_cache/test_SimplePolicy.cc |  4 ++--
 src/tools/immutable_object_cache/ObjectCacheStore.cc | 10 ++++++++--
 src/tools/immutable_object_cache/SimplePolicy.cc     |  2 +-
 6 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/PendingReleaseNotes b/PendingReleaseNotes
index 4d4513e99b383..c4f07b575a653 100644
--- a/PendingReleaseNotes
+++ b/PendingReleaseNotes
@@ -48,6 +48,10 @@
 * An AWS-compliant API: "GetTopicAttributes" was added to replace the existing "GetTopic" API. The new API
   should be used to fetch information about topics used for bucket notifications.
 
+* librbd: The shared, read-only parent cache's config option ``immutable_object_cache_watermark`` now has been updated
+  to property reflect the upper cache utilization before space is reclaimed. The default ``immutable_object_cache_watermark``
+  now is ``0.9``. If the capacity reaches 90% the daemon will delete cold cache.
+
 >=15.0.0
 --------
 
diff --git a/doc/rbd/rbd-persistent-read-only-cache.rst b/doc/rbd/rbd-persistent-read-only-cache.rst
index dbaa66035e3d6..3b17efb9cec92 100644
--- a/doc/rbd/rbd-persistent-read-only-cache.rst
+++ b/doc/rbd/rbd-persistent-read-only-cache.rst
@@ -116,8 +116,8 @@ The ``ceph-immutable-object-cache`` daemon is available within the optional
 .. important:: ``ceph-immutable-object-cache`` daemon requires the ability to
    connect RADOS clusters.
 
-Run Daemon
-----------
+Running the Immutable Object Cache Daemon
+-----------------------------------------
 
 ``ceph-immutable-object-cache`` daemon should use a unique Ceph user ID.
 To `create a Ceph user`_, with ``ceph`` specify the ``auth get-or-create``
@@ -137,7 +137,7 @@ The ``ceph-immutable-object-cache`` can also be run in foreground by ``ceph-immu
 QOS Settings
 ------------
 
-Immutable object cache supports throttle, controlled by the following settings.
+The immutable object cache supports throttling, controlled by the following settings:
 
 ``immutable_object_cache_qos_schedule_tick_min``
 
diff --git a/src/common/options.cc b/src/common/options.cc
index 2dd0e07b8a61f..e3ef3bcb3e0c5 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -7910,7 +7910,7 @@ static std::vector<Option> get_immutable_object_cache_options() {
     .set_description("immutable object cache client dedicated thread number"),
 
     Option("immutable_object_cache_watermark", Option::TYPE_FLOAT, Option::LEVEL_ADVANCED)
-    .set_default(0.1)
+    .set_default(0.9)
     .set_description("immutable object cache water mark"),
 
     Option("immutable_object_cache_qos_schedule_tick_min", Option::TYPE_MILLISECS, Option::LEVEL_ADVANCED)
diff --git a/src/test/immutable_object_cache/test_SimplePolicy.cc b/src/test/immutable_object_cache/test_SimplePolicy.cc
index dee6b5b417bb0..26f503be4f7fc 100644
--- a/src/test/immutable_object_cache/test_SimplePolicy.cc
+++ b/src/test/immutable_object_cache/test_SimplePolicy.cc
@@ -30,7 +30,7 @@ public:
   static void SetUpTestCase() {}
   static void TearDownTestCase() {}
   void SetUp() override {
-    m_simple_policy = new SimplePolicy(g_ceph_context, m_cache_size, 128, 0.1);
+    m_simple_policy = new SimplePolicy(g_ceph_context, m_cache_size, 128, 0.9);
     // populate 50 entries
     for (uint64_t i = 0; i < m_cache_size / 2; i++, m_entry_index++) {
       insert_entry_into_promoted_lru(generate_file_name(m_entry_index));
@@ -210,7 +210,7 @@ TEST_F(TestSimplePolicy, test_update_state_from_promoting_to_promoted) {
 
 TEST_F(TestSimplePolicy, test_evict_list_0) {
   std::list<std::string> evict_entry_list;
-  // 0.1 is watermark
+  // the default water mark is 0.9
   ASSERT_TRUE((float)m_simple_policy->get_free_size() > m_cache_size*0.1);
   m_simple_policy->get_evict_list(&evict_entry_list);
   ASSERT_TRUE(evict_entry_list.size() == 0);
diff --git a/src/tools/immutable_object_cache/ObjectCacheStore.cc b/src/tools/immutable_object_cache/ObjectCacheStore.cc
index 18f64250ff77d..483687ffe5fc9 100644
--- a/src/tools/immutable_object_cache/ObjectCacheStore.cc
+++ b/src/tools/immutable_object_cache/ObjectCacheStore.cc
@@ -88,6 +88,10 @@ ObjectCacheStore::ObjectCacheStore(CephContext *cct)
                    ("immutable_object_cache_qos_bps_burst_seconds"));
   }
 
+  if ((cache_watermark <= 0) || (cache_watermark > 1)) {
+    lderr(m_cct) << "Invalid water mark provided, set it to default." << dendl;
+    cache_watermark = 0.9;
+  }
   m_policy = new SimplePolicy(m_cct, cache_max_size, max_inflight_ops,
                               cache_watermark);
 }
@@ -161,7 +165,8 @@ int ObjectCacheStore::do_promote(std::string pool_nspace, uint64_t pool_id,
                    << " snapshot: " << snap_id << dendl;
 
   int ret = 0;
-  std::string cache_file_name = get_cache_file_name(pool_nspace, pool_id, snap_id, object_name);
+  std::string cache_file_name =
+    get_cache_file_name(pool_nspace, pool_id, snap_id, object_name);
   librados::IoCtx ioctx;
   {
     std::lock_guard _locker{m_ioctx_map_lock};
@@ -247,7 +252,8 @@ int ObjectCacheStore::lookup_object(std::string pool_nspace, uint64_t pool_id,
                    << " in pool ID : " << pool_id << dendl;
 
   int pret = -1;
-  std::string cache_file_name = get_cache_file_name(pool_nspace, pool_id, snap_id, object_name);
+  std::string cache_file_name =
+    get_cache_file_name(pool_nspace, pool_id, snap_id, object_name);
 
   cache_status_t ret = m_policy->lookup_object(cache_file_name);
 
diff --git a/src/tools/immutable_object_cache/SimplePolicy.cc b/src/tools/immutable_object_cache/SimplePolicy.cc
index 7dbd9d34bd9cf..3a7375ba97626 100644
--- a/src/tools/immutable_object_cache/SimplePolicy.cc
+++ b/src/tools/immutable_object_cache/SimplePolicy.cc
@@ -170,7 +170,7 @@ void SimplePolicy::get_evict_list(std::list<std::string>* obj_list) {
 
   std::unique_lock locker{m_cache_map_lock};
   // check free ratio, pop entries from LRU
-  if ((double)m_cache_size / m_max_cache_size > (1 - m_watermark)) {
+  if ((double)m_cache_size > m_max_cache_size * m_watermark) {
     // TODO(dehao): make this configurable
     int evict_num = m_cache_map.size() * 0.1;
     for (int i = 0; i < evict_num; i++) {
-- 
2.39.5