-
Notifications
You must be signed in to change notification settings - Fork 22
Add UseDetectedLocalDC balancing policy #564
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,10 @@ std::pair<NThreading::TFuture<TEndpointUpdateResult>, bool> TEndpointPool::Updat | |
| TListEndpointsResult result = future.GetValue(); | ||
| std::vector<std::string> removed; | ||
| if (result.DiscoveryStatus.Status == EStatus::SUCCESS) { | ||
| if (BalancingPolicy_.PolicyType == TBalancingPolicy::TImpl::EPolicyType::UseDetectedLocalDC) { | ||
| LocalDCDetector_.DetectLocalDC(result.Result); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. А это ведь прям тяжелая операция. А мы ее синхронно делаем. Это может discovery сильно замедлить, тут замеры не помешали бы. Лучше пинги в фоне делать, как по мне, но такое писать сложнее будет |
||
| } | ||
|
|
||
| std::vector<TEndpointRecord> records; | ||
| // Is used to convert float to integer load factor | ||
| // same integer values will be selected randomly. | ||
|
|
@@ -182,6 +186,8 @@ bool TEndpointPool::IsPreferredEndpoint(const Ydb::Discovery::EndpointInfo& endp | |
| return true; | ||
| case TBalancingPolicy::TImpl::EPolicyType::UsePreferableLocation: | ||
| return endpoint.location() == BalancingPolicy_.Location.value_or(selfLocation); | ||
| case TBalancingPolicy::TImpl::EPolicyType::UseDetectedLocalDC: | ||
| return LocalDCDetector_.IsLocalDC(endpoint.location()); | ||
| case TBalancingPolicy::TImpl::EPolicyType::UsePreferablePileState: | ||
| if (auto it = pileStates.find(endpoint.bridge_pile_name()); it != pileStates.end()) { | ||
| return GetPileState(it->second.state()) == BalancingPolicy_.PileState; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| _ydb_sdk_add_library(impl-internal-local_dc_detector) | ||
|
|
||
| target_link_libraries(impl-internal-local_dc_detector PUBLIC | ||
| yutil | ||
| api-protos | ||
| ) | ||
|
|
||
| target_sources(impl-internal-local_dc_detector PRIVATE | ||
| local_dc_detector.cpp | ||
| pinger.cpp | ||
| ) | ||
|
|
||
| _ydb_sdk_install_targets(TARGETS impl-internal-local_dc_detector) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| #define INCLUDE_YDB_INTERNAL_H | ||
| #include "local_dc_detector.h" | ||
|
|
||
| namespace NYdb::inline V3 { | ||
|
|
||
| TLocalDCDetector::TLocalDCDetector(TPinger pingEndpoint) | ||
| : PingEndpoint_(std::move(pingEndpoint)) | ||
| {} | ||
|
|
||
| void TLocalDCDetector::DetectLocalDC(const Ydb::Discovery::ListEndpointsResult& endpointsList) { | ||
| auto endpointsByLocation = GroupEndpointsByLocation(endpointsList); | ||
| SampleEndpoints(endpointsByLocation); | ||
|
|
||
| if (endpointsByLocation.size() > 1) { | ||
| Location_ = FindNearestLocation(endpointsByLocation); | ||
| } else { | ||
| Location_.clear(); | ||
| } | ||
| } | ||
|
|
||
| bool TLocalDCDetector::IsLocalDC(const std::string& location) const { | ||
| return Location_.empty() || Location_ == location; | ||
| } | ||
|
|
||
| TLocalDCDetector::TEndpointsByLocation TLocalDCDetector::GroupEndpointsByLocation(const Ydb::Discovery::ListEndpointsResult& endpointsList) const { | ||
| TEndpointsByLocation endpointsByLocation; | ||
| for (const auto& endpoint : endpointsList.endpoints()) { | ||
| endpointsByLocation[endpoint.location()].emplace_back(endpoint); | ||
| } | ||
| return endpointsByLocation; | ||
| } | ||
|
|
||
| void TLocalDCDetector::SampleEndpoints(TEndpointsByLocation& endpointsByLocation) const { | ||
| std::mt19937 gen(std::random_device{}()); | ||
| for (auto& [location, endpoints] : endpointsByLocation) { | ||
| if (endpoints.size() > MAX_ENDPOINTS_PER_LOCATION) { | ||
| std::vector<TEndpointRef> sample; | ||
| sample.reserve(MAX_ENDPOINTS_PER_LOCATION); | ||
| std::sample(endpoints.begin(), endpoints.end(), std::back_inserter(sample), MAX_ENDPOINTS_PER_LOCATION, gen); | ||
| endpoints.swap(sample); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| std::uint64_t TLocalDCDetector::MeasureLocationRtt(const std::vector<TEndpointRef>& endpoints) const { | ||
| if (endpoints.empty()) { | ||
| return std::numeric_limits<std::uint64_t>::max(); | ||
| } | ||
|
|
||
| std::vector<std::uint64_t> timings; | ||
| timings.reserve(PING_COUNT); | ||
| for (size_t i = 0; i < PING_COUNT; ++i) { | ||
| const auto& ep = endpoints[i % endpoints.size()].get(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Тут кажется проще каждый эндпоинт по PING_COUNT раз пинговать, читаемее будет |
||
| timings.push_back(PingEndpoint_(ep).MicroSeconds()); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. А это же синхронные пинги из одного потока? На первый взгляд так нельзя делать: |
||
| } | ||
| std::sort(timings.begin(), timings.end()); | ||
|
|
||
| return std::midpoint(timings[(PING_COUNT - 1) / 2], timings[PING_COUNT / 2]); | ||
| } | ||
|
|
||
|
|
||
| std::string TLocalDCDetector::FindNearestLocation(const TEndpointsByLocation& endpointsByLocation) { | ||
| auto minRtt = std::numeric_limits<std::uint64_t>::max(); | ||
| std::string nearestLocation; | ||
| for (const auto& [location, endpoints] : endpointsByLocation) { | ||
| auto rtt = MeasureLocationRtt(endpoints); | ||
| if (rtt < minRtt) { | ||
| minRtt = rtt; | ||
| nearestLocation = location; | ||
| } | ||
| } | ||
| return nearestLocation; | ||
| } | ||
|
|
||
| } // namespace NYdb | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| #pragma once | ||
|
|
||
| #include <src/api/protos/ydb_discovery.pb.h> | ||
| #include <src/client/impl/internal/internal_header.h> | ||
| #include <src/client/impl/internal/local_dc_detector/pinger.h> | ||
|
|
||
| #include <algorithm> | ||
| #include <map> | ||
| #include <numeric> | ||
| #include <random> | ||
| #include <string> | ||
| #include <vector> | ||
|
|
||
| namespace NYdb::inline V3 { | ||
|
|
||
| class TLocalDCDetector { | ||
| public: | ||
| using TPinger = std::function<TDuration(const Ydb::Discovery::EndpointInfo& endpoint)>; | ||
| explicit TLocalDCDetector(TPinger pingEndpoint = PingEndpoint); | ||
|
|
||
| void DetectLocalDC(const Ydb::Discovery::ListEndpointsResult& endpoints); | ||
| bool IsLocalDC(const std::string& location) const; | ||
|
|
||
| private: | ||
| using TEndpoint = Ydb::Discovery::EndpointInfo; | ||
| using TEndpointRef = std::reference_wrapper<const TEndpoint>; | ||
| using TEndpointsByLocation = std::unordered_map<std::string, std::vector<TEndpointRef>>; | ||
|
|
||
| TEndpointsByLocation GroupEndpointsByLocation(const Ydb::Discovery::ListEndpointsResult& endpointsList) const; | ||
| void SampleEndpoints(TEndpointsByLocation& endpointsByLocation) const; | ||
| std::uint64_t MeasureLocationRtt(const std::vector<TEndpointRef>& endpoints) const; | ||
| std::string FindNearestLocation(const TEndpointsByLocation& endpointsByLocation); | ||
|
|
||
| private: | ||
| static constexpr std::size_t MAX_ENDPOINTS_PER_LOCATION = 3; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. С одной стороны, маловато нод, но тогда сильно тяжелее будет операция. Может действительно надо асинхронно |
||
| static constexpr std::size_t PING_COUNT = 2 * MAX_ENDPOINTS_PER_LOCATION; | ||
|
|
||
| TPinger PingEndpoint_; | ||
| std::string Location_; | ||
| }; | ||
|
|
||
| } // namespace NYdb | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| #define INCLUDE_YDB_INTERNAL_H | ||
| #include "pinger.h" | ||
|
|
||
| namespace NYdb::inline V3 { | ||
|
|
||
| TDuration PingEndpoint(const Ydb::Discovery::EndpointInfo& endpoint) { | ||
| try { | ||
| TNetworkAddress addr(endpoint.address().data(), static_cast<ui16>(endpoint.port())); | ||
| auto start = TInstant::Now(); | ||
| TSocket sock(addr, TDuration::Seconds(PING_TIMEOUT_SECONDS)); | ||
| return TInstant::Now() - start; | ||
| } catch (...) { | ||
| return TDuration::Max(); | ||
| } | ||
| } | ||
|
|
||
| } // namespace NYdb |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| #pragma once | ||
|
|
||
| #include <src/client/impl/internal/internal_header.h> | ||
| #include <src/api/protos/ydb_discovery.pb.h> | ||
|
|
||
| #include <util/network/sock.h> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Это инклюд можно в .cpp унести |
||
| #include <util/datetime/base.h> | ||
|
|
||
| namespace NYdb::inline V3 { | ||
|
|
||
| static constexpr std::uint32_t PING_TIMEOUT_SECONDS = 5; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Тут лучше временной тип использовать, а не в сырую хранить |
||
|
|
||
| TDuration PingEndpoint(const Ydb::Discovery::EndpointInfo& endpoint); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Лучше тут std::chrono::duration сделать, мы от TDuration и TInstant потихоньку избавляемся в SDK |
||
|
|
||
| } // namespace NYdb | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Распиши чуть подробнее, чем этот вариант от предыдущего отличается. И какие предостережения к использованию (если есть)