From e2429a32cad22f117bc428da385ba731479e88dd Mon Sep 17 00:00:00 2001 From: cailiuyang Date: Thu, 28 Dec 2017 18:20:23 +0800 Subject: [PATCH] fix the drift of the start time of backup policy Summary: Ref T9978 Test Plan: N/A Reviewers: qinzuoyan, sunweijie, heyuchen, wutao1, laiyingchun Reviewed By: sunweijie Subscribers: #pegasus Maniphest Tasks: T9978 Differential Revision: https://phabricator.d.xiaomi.net/D79520 Conflicts: src/tests/dsn/fds_service_test.cpp --- .../meta_server/meta_backup_service.cpp | 77 +++++++++---- .../meta_server/meta_backup_service.h | 32 +++-- .../test/meta_test/unit_test/backup_test.cpp | 109 ++++++++++++++++++ 3 files changed, 190 insertions(+), 28 deletions(-) diff --git a/src/dist/replication/meta_server/meta_backup_service.cpp b/src/dist/replication/meta_server/meta_backup_service.cpp index 86b7f9869f..bd86f379a9 100644 --- a/src/dist/replication/meta_server/meta_backup_service.cpp +++ b/src/dist/replication/meta_server/meta_backup_service.cpp @@ -644,6 +644,60 @@ void policy_context::continue_current_backup_unlocked() } } +bool policy_context::should_start_backup_unlocked() +{ + uint64_t now = dsn_now_ms(); + uint64_t recent_backup_start_time_ms = 0; + if (!_backup_history.empty()) { + recent_backup_start_time_ms = _backup_history.rbegin()->second.start_time_ms; + } + + // the true start time of recent backup have drifted away with the origin start time of + // policy, + // so we should take the drift into consideration; if user change the start time of the + // policy, + // we just think the change of start time as drift + int32_t hour = 0, min = 0, sec = 0; + if (recent_backup_start_time_ms == 0) { + // the first time to backup, just consider the start time + ::dsn::utils::time_ms_to_date_time(now, hour, min, sec); + return _policy.start_time.should_start_backup(hour, min); + } else { + uint64_t next_backup_time_ms = + recent_backup_start_time_ms + _policy.backup_interval_seconds * 1000; + if (_policy.start_time.hour != 24) { + // user have specify the time point to start backup, so we should take the the + // time-drift into consideration + + // compute the time-drift + ::dsn::utils::time_ms_to_date_time(recent_backup_start_time_ms, hour, min, sec); + int64_t time_dirft_ms = _policy.start_time.compute_time_drift_ms(hour, min); + + if (time_dirft_ms >= 0) { + // hour:min(the true start time) >= policy.start_time : + // 1, user move up the start time of policy, such as 20:00 to 2:00, we just + // think this case as time drift + // 2, the true start time of backup is delayed, compared the origin start time + // of policy, we should process this case + // 3, the true start time of backup is the same with the origin start time of + // policy + next_backup_time_ms -= time_dirft_ms; + } else { + // hour:min(the true start time) < policy.start_time: + // 1, user delay the start time of policy, such as 2:00 to 23:00 + // + // these case has already been handled, we do nothing + } + } + if (next_backup_time_ms <= now) { + ::dsn::utils::time_ms_to_date_time(now, hour, min, sec); + return _policy.start_time.should_start_backup(hour, min); + } else { + return false; + } + } +} + void policy_context::issue_new_backup_unlocked() { // before issue new backup, we check whether the policy is dropped @@ -661,27 +715,8 @@ void policy_context::issue_new_backup_unlocked() std::chrono::milliseconds(_backup_service->backup_option().issue_backup_interval_ms)); return; } - uint64_t now = dsn_now_ms(); - int32_t hour = 0, min = 0, sec = 0; - dsn::utils::time_ms_to_date_time(now, hour, min, sec); - bool issue_backup_later = false; - if (!_backup_history.empty()) { - const backup_info &recent_backup = _backup_history.rbegin()->second; - uint64_t next_backup_moment = - recent_backup.start_time_ms + _policy.backup_interval_seconds * 1000; - if (next_backup_moment > now) { - issue_backup_later = true; - } else if (!_policy.start_time.should_start_backup(hour, min, sec)) { - issue_backup_later = true; - } - } else { - if (!_policy.start_time.should_start_backup(hour, min, sec)) { - issue_backup_later = true; - } else { - ddebug("%s: start first backup for this policy", _policy.policy_name.c_str()); - } - } - if (issue_backup_later) { + + if (!should_start_backup_unlocked()) { tasking::enqueue( LPC_DEFAULT_CALLBACK, nullptr, diff --git a/src/dist/replication/meta_server/meta_backup_service.h b/src/dist/replication/meta_server/meta_backup_service.h index d63ec94b08..b6a2d264d2 100644 --- a/src/dist/replication/meta_server/meta_backup_service.h +++ b/src/dist/replication/meta_server/meta_backup_service.h @@ -57,6 +57,9 @@ struct backup_info // Attention: backup_start_time == 24:00 is represent no limit for start_time, 24:00 is mainly saved // for testing +// +// current, we don't support accurating to minute, only support accurating to hour, so +// we just set minute to 0 struct backup_start_time { int32_t hour; // [0 ~24) @@ -95,20 +98,32 @@ struct backup_start_time } return true; } + + // return the interval between new_hour:new_min and start_time, + // namely new_hour:new_min - start_time; + // unit is ms + int64_t compute_time_drift_ms(int32_t new_hour, int32_t new_min) + { + int64_t res = 0; + // unit is hour + res += (new_hour - hour); + // unit is minute + res *= 60; + res += (new_min - minute); + // unit is ms + return (res * 60 * 1000); + } + // judge whether we should start backup base current time - bool should_start_backup(int32_t cur_hour, int32_t cur_min, int32_t cur_sec) + bool should_start_backup(int32_t cur_hour, int32_t cur_min) { if (hour == 24) { // erase the restrict of backup_start_time, just for testing return true; } - // NOTICE : if you want more precisely, you can use cur_min and cur_sec to implement + // NOTICE : if you want more precisely, you can use cur_min to implement // now, we just ignore - if (cur_hour == hour) { - return true; - } else { - return false; - } + return (cur_hour == hour); } DEFINE_JSON_SERIALIZATION(hour, minute) }; @@ -237,6 +252,9 @@ mock_private : mock_virtual void initialize_backup_progress_unlocked(); mock_virtual void prepare_current_backup_on_new_unlocked(); mock_virtual void issue_new_backup_unlocked(); + // returns: + // - true, should start backup right now, otherwise don't start backup + mock_virtual bool should_start_backup_unlocked(); mock_virtual void continue_current_backup_unlocked(); mock_virtual void on_backup_reply(dsn::error_code err, diff --git a/src/dist/replication/test/meta_test/unit_test/backup_test.cpp b/src/dist/replication/test/meta_test/unit_test/backup_test.cpp index 52d88e018e..7d2ae6283c 100644 --- a/src/dist/replication/test/meta_test/unit_test/backup_test.cpp +++ b/src/dist/replication/test/meta_test/unit_test/backup_test.cpp @@ -518,6 +518,115 @@ void meta_service_test_app::policy_context_test() ->wait(); ASSERT_EQ(dsn::ERR_OK, ec); } + + // test should_start_backup_unlock() + { + std::cout << "test should_start_backup_unlock()" << std::endl; + uint64_t now = dsn_now_ms(); + int32_t hour = 0, min = 0, sec = 0; + ::dsn::utils::time_ms_to_date_time(now, hour, min, sec); + while (min == 59) { + std::this_thread::sleep_for(std::chrono::minutes(1)); + now = dsn_now_ms(); + ::dsn::utils::time_ms_to_date_time(now, hour, min, sec); + } + + int64_t oneday_sec = 1 * 24 * 60 * 60; + mp._policy.start_time.hour = hour; + mp._policy.start_time.minute = 0; + mp._policy.backup_interval_seconds = oneday_sec; // oneday + mp._backup_history.clear(); + + backup_info info; + + { + std::cout << "first backup & no limit to start_time" << std::endl; + mp._policy.start_time.hour = 24; + ASSERT_TRUE(mp.should_start_backup_unlocked()); + } + + { + std::cout << "first backup & cur_time.hour == start_time.hour" << std::endl; + ASSERT_TRUE(mp.should_start_backup_unlocked()); + } + + { + + std::cout << "first backup & cur_time.hour != start_time.hour" << std::endl; + mp._policy.start_time.hour = hour + 100; + ASSERT_FALSE(mp.should_start_backup_unlocked()); + mp._policy.start_time.hour = 25 - hour; + ASSERT_FALSE(mp.should_start_backup_unlocked()); + } + + { + std::cout << "not first backup & recent backup delay 20min to start" << std::endl; + info.start_time_ms = now - (oneday_sec * 1000) + 20 * 60 * 1000; + info.end_time_ms = info.start_time_ms + 10; + mp.add_backup_history(info); + // if we set start_time to 24:00, then will not start backup + mp._policy.start_time.hour = 24; + ASSERT_FALSE(mp.should_start_backup_unlocked()); + // if we set start_time to hour:00, then will start backup, even if the interval < + // policy.backup_interval + mp._policy.start_time.hour = hour; + ASSERT_TRUE(mp.should_start_backup_unlocked()); + } + + { + std::cout << "not first backup & recent backup start time is equal with start_time" + << std::endl; + mp._policy.start_time.hour = hour; + mp._backup_history.clear(); + info.start_time_ms = now - (oneday_sec * 1000) - (min * 60 * 1000); + info.start_time_ms = (info.start_time_ms / 1000) * 1000; + info.end_time_ms = info.start_time_ms + 10; + mp.add_backup_history(info); + ASSERT_TRUE(mp.should_start_backup_unlocked()); + } + + { + // delay the start_time + std::cout << "not first backup & delay the start time of policy" << std::endl; + mp._policy.start_time.hour = hour + 1; + mp._backup_history.clear(); + // make sure the start time of recent backup is litte than policy's start_time, so we + // minus more 3min + info.start_time_ms = now - (oneday_sec * 1000) - 3 * 60 * 1000; + info.end_time_ms = info.start_time_ms + 10; + mp.add_backup_history(info); + if (mp._policy.start_time.hour == 24) { + // if hour = 23, then policy.start_time.hour = 24, we should start next backup, + // because now - info.start_time_ms > policy.backup_interval + ASSERT_TRUE(mp.should_start_backup_unlocked()); + } else { + // should not start, even if now - info.start_time_ms > policy.backup_interval, but + // not reach the time-point that policy.start_time limit + ASSERT_FALSE(mp.should_start_backup_unlocked()); + } + } + + { + std::cout << "not first backup & no limit to start time & should start backup" + << std::endl; + mp._policy.start_time.hour = 24; + mp._backup_history.clear(); + info.start_time_ms = now - (oneday_sec * 1000) - 3 * 60 * 60; + info.end_time_ms = info.start_time_ms + 10; + mp.add_backup_history(info); + ASSERT_TRUE(mp.should_start_backup_unlocked()); + } + + { + std::cout << "not first backup & no limit to start time & should not start backup" + << std::endl; + mp._backup_history.clear(); + info.start_time_ms = now - (oneday_sec * 1000) + 3 * 60 * 60; + info.end_time_ms = info.start_time_ms + 10; + mp.add_backup_history(info); + ASSERT_FALSE(mp.should_start_backup_unlocked()); + } + } } void meta_service_test_app::backup_service_test()