From 5ff127736bc10d657ccdee9aabf179bbfa81d5c0 Mon Sep 17 00:00:00 2001 From: "J. Eric Ivancich" Date: Wed, 13 Sep 2017 15:48:19 -0400 Subject: [PATCH] Squashed 'src/dmclock/' changes from 3408cb8f3c..a9e777f08f a9e777f08f Merge pull request #35 from joscollin/wip-warning-dmclock 45c63a5b10 Merge pull request #36 from ceph/wip-fix-functional 40a9fe55a6 With upgrade to gcc 7.1.1 need to include in additional header files. a2dd155f08 src/dmclock_server.h: silence warning from -Wmaybe-uninitialized 0d6c7c6544 Merge pull request #24 from bspark8/wip_online_client_info_f a5e8cdea31 Add unit test for update_client_info, dynamic_cli_info_f. 1faf3524c6 Modify each client's QoS parameter applied time from client_rec creation time to each request's tagging creation time. git-subtree-dir: src/dmclock git-subtree-split: a9e777f08f288fe6834599afdeefcd9c7d31e6c1 --- sim/src/sim_recs.h | 1 + sim/src/ssched/ssched_server.h | 1 + src/dmclock_server.h | 72 +++++++++---- support/src/run_every.h | 1 + test/test_dmclock_server.cc | 185 +++++++++++++++++++++++++++++++++ 5 files changed, 238 insertions(+), 22 deletions(-) diff --git a/sim/src/sim_recs.h b/sim/src/sim_recs.h index b64750db4afac..759ab4e14134e 100644 --- a/sim/src/sim_recs.h +++ b/sim/src/sim_recs.h @@ -21,6 +21,7 @@ #include #include #include +#include using ClientId = uint; diff --git a/sim/src/ssched/ssched_server.h b/sim/src/ssched/ssched_server.h index 610c2ef665c43..fcc7055450a4b 100644 --- a/sim/src/ssched/ssched_server.h +++ b/sim/src/ssched/ssched_server.h @@ -10,6 +10,7 @@ #include #include #include +#include #include "boost/variant.hpp" diff --git a/src/dmclock_server.h b/src/dmclock_server.h index 2c9940dc6c17f..aac848746c0ca 100644 --- a/src/dmclock_server.h +++ b/src/dmclock_server.h @@ -66,15 +66,15 @@ namespace crimson { constexpr uint tag_modulo = 1000000; struct ClientInfo { - const double reservation; // minimum - const double weight; // proportional - const double limit; // maximum + double reservation; // minimum + double weight; // proportional + double limit; // maximum // multiplicative inverses of above, which we use in calculations // and don't want to recalculate repeatedly - const double reservation_inv; - const double weight_inv; - const double limit_inv; + double reservation_inv; + double weight_inv; + double limit_inv; // order parameters -- min, "normal", max ClientInfo(double _reservation, double _weight, double _limit) : @@ -229,9 +229,10 @@ namespace crimson { }; // class RequestTag - // C is client identifier type, R is request type, B is heap - // branching factor - template + // C is client identifier type, R is request type, + // U1 determines whether to use client information function dynamically, + // B is heap branching factor + template class PriorityQueueBase { // we don't want to include gtest.h just for FRIEND_TEST friend class dmclock_server_client_idle_erase_Test; @@ -285,7 +286,7 @@ namespace crimson { // ClientRec could be "protected" with no issue. [See comments // associated with function submit_top_request.] class ClientRec { - friend PriorityQueueBase; + friend PriorityQueueBase; C client; RequestTag prev_tag; @@ -414,7 +415,7 @@ namespace crimson { friend std::ostream& operator<<(std::ostream& out, - const typename PriorityQueueBase::ClientRec& e) { + const typename PriorityQueueBase::ClientRec& e) { out << "{ ClientRec::" << " client:" << e.client << " prev_tag:" << e.prev_tag << @@ -543,6 +544,24 @@ namespace crimson { } + void update_client_info(const C& client_id) { + DataGuard g(data_mtx); + auto client_it = client_map.find(client_id); + if (client_map.end() != client_it) { + ClientRec& client = (*client_it->second); + client.info = client_info_f(client_id); + } + } + + + void update_client_infos() { + DataGuard g(data_mtx); + for (auto i : client_map) { + i.second->info = client_info_f(i.second->client); + } + } + + friend std::ostream& operator<<(std::ostream& out, const PriorityQueueBase& q) { std::lock_guard guard(q.data_mtx); @@ -651,7 +670,8 @@ namespace crimson { } }; - ClientInfoFunc client_info_f; + ClientInfoFunc client_info_f; + static constexpr bool is_dynamic_cli_info_f = U1; mutable std::mutex data_mtx; using DataGuard = std::lock_guard; @@ -744,6 +764,14 @@ namespace crimson { } + inline const ClientInfo get_cli_info(ClientRec& client) const { + if (is_dynamic_cli_info_f) { + client.info = client_info_f(client.client); + } + return client.info; + } + + // data_mtx must be held by caller void do_add_request(RequestRef&& request, const C& client_id, @@ -831,7 +859,7 @@ namespace crimson { if (!client.has_request()) { tag = RequestTag(client.get_req_tag(), - client.info, + get_cli_info(client), req_params, time, cost); @@ -840,7 +868,7 @@ namespace crimson { client.update_req_tag(tag, tick); } #else - RequestTag tag(client.get_req_tag(), client.info, req_params, time, cost); + RequestTag tag(client.get_req_tag(), get_cli_info(client), req_params, time, cost); // copy tag to previous tag for client client.update_req_tag(tag, tick); #endif @@ -890,7 +918,7 @@ namespace crimson { #ifndef DO_NOT_DELAY_TAG_CALC if (top.has_request()) { ClientReq& next_first = top.next_request(); - next_first.tag = RequestTag(tag, top.info, + next_first.tag = RequestTag(tag, get_cli_info(top), top.cur_delta, top.cur_rho, next_first.tag.arrival); @@ -940,7 +968,7 @@ namespace crimson { // data_mtx should be held when called NextReq do_next_request(Time now) { - NextReq result; + NextReq result{}; // if reservation queue is empty, all are empty (i.e., no active clients) if(resv_heap.empty()) { @@ -1102,9 +1130,9 @@ namespace crimson { }; // class PriorityQueueBase - template - class PullPriorityQueue : public PriorityQueueBase { - using super = PriorityQueueBase; + template + class PullPriorityQueue : public PriorityQueueBase { + using super = PriorityQueueBase; public: @@ -1318,12 +1346,12 @@ namespace crimson { // PUSH version - template - class PushPriorityQueue : public PriorityQueueBase { + template + class PushPriorityQueue : public PriorityQueueBase { protected: - using super = PriorityQueueBase; + using super = PriorityQueueBase; public: diff --git a/support/src/run_every.h b/support/src/run_every.h index 494db92e31657..98b25d857c74d 100644 --- a/support/src/run_every.h +++ b/support/src/run_every.h @@ -11,6 +11,7 @@ #include #include #include +#include namespace crimson { diff --git a/test/test_dmclock_server.cc b/test/test_dmclock_server.cc index 95def410fb69f..1d74e58683b82 100644 --- a/test/test_dmclock_server.cc +++ b/test/test_dmclock_server.cc @@ -639,6 +639,191 @@ namespace crimson { } // dmclock_server_pull.pull_reservation + TEST(dmclock_server_pull, update_client_info) { + using ClientId = int; + using Queue = dmc::PullPriorityQueue; + using QueueRef = std::unique_ptr; + + ClientId client1 = 17; + ClientId client2 = 98; + + dmc::ClientInfo info1(0.0, 100.0, 0.0); + dmc::ClientInfo info2(0.0, 200.0, 0.0); + + QueueRef pq; + + auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo { + if (client1 == c) return info1; + else if (client2 == c) return info2; + else { + ADD_FAILURE() << "client info looked up for non-existant client"; + return info1; + } + }; + + pq = QueueRef(new Queue(client_info_f, false)); + + ReqParams req_params(1,1); + + auto now = dmc::get_time(); + + for (int i = 0; i < 5; ++i) { + pq->add_request(Request{}, client1, req_params); + pq->add_request(Request{}, client2, req_params); + now += 0.0001; + } + + int c1_count = 0; + int c2_count = 0; + for (int i = 0; i < 10; ++i) { + Queue::PullReq pr = pq->pull_request(); + EXPECT_EQ(Queue::NextReqType::returning, pr.type); + auto& retn = boost::get(pr.data); + + if (i > 5) continue; + if (client1 == retn.client) ++c1_count; + else if (client2 == retn.client) ++c2_count; + else ADD_FAILURE() << "got request from neither of two clients"; + + EXPECT_EQ(PhaseType::priority, retn.phase); + } + + EXPECT_EQ(2, c1_count) << + "before: one-third of request should have come from first client"; + EXPECT_EQ(4, c2_count) << + "before: two-thirds of request should have come from second client"; + + std::chrono::seconds dura(1); + std::this_thread::sleep_for(dura); + + info1 = dmc::ClientInfo(0.0, 200.0, 0.0); + pq->update_client_info(17); + + now = dmc::get_time(); + + for (int i = 0; i < 5; ++i) { + pq->add_request(Request{}, client1, req_params); + pq->add_request(Request{}, client2, req_params); + now += 0.0001; + } + + c1_count = 0; + c2_count = 0; + for (int i = 0; i < 6; ++i) { + Queue::PullReq pr = pq->pull_request(); + EXPECT_EQ(Queue::NextReqType::returning, pr.type); + auto& retn = boost::get(pr.data); + + if (client1 == retn.client) ++c1_count; + else if (client2 == retn.client) ++c2_count; + else ADD_FAILURE() << "got request from neither of two clients"; + + EXPECT_EQ(PhaseType::priority, retn.phase); + } + + EXPECT_EQ(3, c1_count) << + "after: one-third of request should have come from first client"; + EXPECT_EQ(3, c2_count) << + "after: two-thirds of request should have come from second client"; + } + + + TEST(dmclock_server_pull, dynamic_cli_info_f) { + using ClientId = int; + using Queue = dmc::PullPriorityQueue; + using QueueRef = std::unique_ptr; + + ClientId client1 = 17; + ClientId client2 = 98; + + std::vector info1; + std::vector info2; + + info1.push_back(dmc::ClientInfo(0.0, 100.0, 0.0)); + info1.push_back(dmc::ClientInfo(0.0, 150.0, 0.0)); + + info2.push_back(dmc::ClientInfo(0.0, 200.0, 0.0)); + info2.push_back(dmc::ClientInfo(0.0, 50.0, 0.0)); + + uint cli_info_group = 0; + + QueueRef pq; + + auto client_info_f = [&] (ClientId c) -> dmc::ClientInfo { + if (client1 == c) return info1[cli_info_group]; + else if (client2 == c) return info2[cli_info_group]; + else { + ADD_FAILURE() << "client info looked up for non-existant client"; + return info1[0]; + } + }; + + pq = QueueRef(new Queue(client_info_f, false)); + + ReqParams req_params(1,1); + + auto now = dmc::get_time(); + + for (int i = 0; i < 5; ++i) { + pq->add_request(Request{}, client1, req_params); + pq->add_request(Request{}, client2, req_params); + now += 0.0001; + } + + int c1_count = 0; + int c2_count = 0; + for (int i = 0; i < 10; ++i) { + Queue::PullReq pr = pq->pull_request(); + EXPECT_EQ(Queue::NextReqType::returning, pr.type); + auto& retn = boost::get(pr.data); + + if (i > 5) continue; + if (client1 == retn.client) ++c1_count; + else if (client2 == retn.client) ++c2_count; + else ADD_FAILURE() << "got request from neither of two clients"; + + EXPECT_EQ(PhaseType::priority, retn.phase); + } + + EXPECT_EQ(2, c1_count) << + "before: one-third of request should have come from first client"; + EXPECT_EQ(4, c2_count) << + "before: two-thirds of request should have come from second client"; + + std::chrono::seconds dura(1); + std::this_thread::sleep_for(dura); + + cli_info_group = 1; + + now = dmc::get_time(); + + for (int i = 0; i < 6; ++i) { + pq->add_request(Request{}, client1, req_params); + pq->add_request(Request{}, client2, req_params); + now += 0.0001; + } + + c1_count = 0; + c2_count = 0; + for (int i = 0; i < 8; ++i) { + Queue::PullReq pr = pq->pull_request(); + EXPECT_EQ(Queue::NextReqType::returning, pr.type); + auto& retn = boost::get(pr.data); + + if (client1 == retn.client) ++c1_count; + else if (client2 == retn.client) ++c2_count; + else ADD_FAILURE() << "got request from neither of two clients"; + + EXPECT_EQ(PhaseType::priority, retn.phase); + } + + EXPECT_EQ(6, c1_count) << + "after: one-third of request should have come from first client"; + EXPECT_EQ(2, c2_count) << + "after: two-thirds of request should have come from second client"; + } + + // This test shows what happens when a request can be ready (under // limit) but not schedulable since proportion tag is 0. We expect // to get some future and none responses. -- 2.39.5