Skip to content
This repository was archived by the owner on Oct 23, 2024. It is now read-only.
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 36 additions & 23 deletions src/contention_detectors/overload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,39 +5,36 @@

#include "mesos/resources.hpp"

#include "serenity/resource_helper.hpp"

namespace mesos {
namespace serenity {

Try<Nothing> OverloadDetector::consume(const ResourceUsage& in) {
void OverloadDetector::allProductsReady() {
Contentions product;
ResourceUsage usage = getConsumable().get();

if (in.total_size() == 0) {
return Error(std::string(NAME) + " No total in ResourceUsage");
if (usage.total_size() == 0) {
SERENITY_LOG(ERROR) << std::string(NAME) << " No total in ResourceUsage";
produce(Contentions());
}

Resources totalAgentResources(in.total());
Resources totalAgentResources(usage.total());
Option<double_t> totalAgentCpus = totalAgentResources.cpus();

if (totalAgentCpus.isNone()) {
return Error(std::string(NAME) + " No total cpus in ResourceUsage");
SERENITY_LOG(ERROR) << std::string(NAME)
<< " No total cpus in ResourceUsage";
produce(product);
}

double_t thresholdCpus = this->cfgUtilizationThreshold * totalAgentCpus.get();
double_t agentSumCpus = 0;
uint64_t beExecutors = 0;

for (const ResourceUsage_Executor& inExec : in.executors()) {
if (!inExec.has_executor_info()) {
SERENITY_LOG(ERROR) << "Executor <unknown>"
<< " does not include executor_info";
// Filter out these executors.
continue;
}
if (!inExec.has_statistics()) {
SERENITY_LOG(ERROR) << "Executor "
<< inExec.executor_info().executor_id().value()
<< " does not include statistics.";
// Filter out these executors.
for (const ResourceUsage_Executor& inExec : usage.executors()) {
// Validate for statistics and executor info.
if (!hasRequiredFields(inExec)) {
continue;
}

Expand All @@ -49,13 +46,14 @@ Try<Nothing> OverloadDetector::consume(const ResourceUsage& in) {

agentSumCpus += value.get();

if (!Resources(inExec.allocated()).revocable().empty()) {
if (ResourceUsageHelper::isRevocableExecutor(inExec)) {
beExecutors++;
}
}

SERENITY_LOG(INFO) << "Sum = " << agentSumCpus << " vs total = "
<< totalAgentCpus.get() << " [threshold = " << thresholdCpus << "]";
<< totalAgentCpus.get() << " [threshold = "
<< thresholdCpus << "]";

if (agentSumCpus > thresholdCpus) {
if (beExecutors == 0) {
Expand All @@ -70,12 +68,27 @@ Try<Nothing> OverloadDetector::consume(const ResourceUsage& in) {
}
}

// Continue pipeline.
this->produce(product);

return Nothing();
produce(product);
}

bool OverloadDetector::hasRequiredFields(const ResourceUsage_Executor& inExec) {
if (!inExec.has_executor_info()) {
SERENITY_LOG(ERROR) << "Executor <unknown>"
<< " does not include executor_info";
// Filter out these executors.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment no longer apply here. And bellow.
Comments lie all the times.

return false;
}

if (!inExec.has_statistics()) {
SERENITY_LOG(ERROR) << "Executor "
<< inExec.executor_info().executor_id().value()
<< " does not include statistics.";
// Filter out these executors.
return false;
}

return true;
}

} // namespace serenity
} // namespace mesos
34 changes: 12 additions & 22 deletions src/contention_detectors/overload.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,6 @@
namespace mesos {
namespace serenity {

class OverloadDetectorConfig : public SerenityConfig {
public:
OverloadDetectorConfig() { }

explicit OverloadDetectorConfig(const SerenityConfig& customCfg) {
this->initDefaults();
this->applyConfig(customCfg);
}

void initDefaults() {
//! double_t
//! Detector threshold.
this->fields[detector::THRESHOLD] =
detector::DEFAULT_UTILIZATION_THRESHOLD;
}
};

/**
* OverloadDetector is able to create contention if utilization is above
Expand All @@ -48,23 +32,29 @@ class OverloadDetector :
OverloadDetector(
Consumer<Contentions>* _consumer,
const lambda::function<usage::GetterFunction>& _cpuUsageGetFunction,
SerenityConfig _conf,
const SerenityConfig& _conf,
const Tag& _tag = Tag(QOS_CONTROLLER, NAME))
: tag(_tag),
cpuUsageGetFunction(_cpuUsageGetFunction),
Producer<Contentions>(_consumer) {
SerenityConfig config = OverloadDetectorConfig(_conf);
this->cfgUtilizationThreshold =
config.getD(detector::THRESHOLD);
// Parse config values.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is rather self-explanatory.

setCfgUtilizationThreshold(
_conf.item<double_t>(detector::THRESHOLD,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

detector::THRESHOLD should be named UTILIZATION_THRESHOLD_KEY and declared as static constexpr in OverloadDetector. Similar for DEFAULT_UTILIZATION_THRESHOLD.

detector::DEFAULT_UTILIZATION_THRESHOLD));
}

~OverloadDetector() {}

Try<Nothing> consume(const ResourceUsage& in) override;

static const constexpr char* NAME = "OverloadDetector";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field should be below functions, right?


void setCfgUtilizationThreshold(double_t cfgUtilizationThreshold) {
OverloadDetector::cfgUtilizationThreshold = cfgUtilizationThreshold;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/s/cfgUtilizationThreshold/utilizationThreshold

}

protected:
void allProductsReady() override;
bool hasRequiredFields(const ResourceUsage_Executor& inExec);

const Tag tag;
const lambda::function<usage::GetterFunction> cpuUsageGetFunction;

Expand Down
131 changes: 70 additions & 61 deletions src/contention_detectors/signal_analyzers/drop.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,56 +30,6 @@ namespace serenity {

#define SIGNAL_DROP_ANALYZER_NAME "AssuranceDropAnalyzer"

class SignalDropAnalyzerConfig : public SerenityConfig {
public:
SignalDropAnalyzerConfig() {}

explicit SignalDropAnalyzerConfig(const SerenityConfig& customCfg) {
this->initDefaults();
this->applyConfig(customCfg);
}

void initDefaults() {
this->fields[detector::ANALYZER_TYPE] = SIGNAL_DROP_ANALYZER_NAME;
//! uint64_t
//! How far in the past we look.
this->fields[detector::WINDOW_SIZE] =
detector::DEFAULT_WINDOW_SIZE;

//! double_t
//! Defines how much (relatively to base point) value must drop to trigger
//! contention.
//! Most signal_analyzer will use that.
this->fields[detector::FRACTIONAL_THRESHOLD] =
detector::DEFAULT_FRACTIONAL_THRESHOLD;

//! double_t
//! You can adjust how big severity is created for a defined drop.
//! if -1 then unknown severity will be reported.
this->fields[detector::SEVERITY_FRACTION] = (double_t) -1;

//! double_t
//! Tolerance fraction of threshold if signal is accepted as returned to
//! previous state after drop.
this->fields[detector::NEAR_FRACTION] =
detector::DEFAULT_NEAR_FRACTION;

//! uint64_t
//! Maximum number of checkpoints we will have in our assurance detector.
//! Checkpoints are the reference assurance_test(base) points which we refer
//! to in the past when detecting drop or not.
//! It needs to be 0 < < WINDOW_SIZE
this->fields[detector::MAX_CHECKPOINTS] =
detector::DEFAULT_MAX_CHECKPOINTS;

//! double_t
//! Fraction of checkpoints' votes that important decision needs to obtain.
this->fields[detector::QUORUM] =
detector::DEFAULT_QUORUM;
}
};


/**
* Dynamic implementation of sequential change point detection.
*
Expand Down Expand Up @@ -109,13 +59,29 @@ class SignalDropAnalyzer : public SignalAnalyzer {
: SignalAnalyzer(_tag),
valueBeforeDrop(None()),
quorumNum(0) {
SerenityConfig config = SignalDropAnalyzerConfig(_config);
this->cfgWindowSize = config.getU64(detector::WINDOW_SIZE);
this->cfgMaxCheckpoints = config.getU64(detector::MAX_CHECKPOINTS);
this->cfgQuroum = config.getD(detector::QUORUM);
this->cfgFractionalThreshold = config.getD(detector::FRACTIONAL_THRESHOLD);
this->cfgNearFraction = config.getD(detector::NEAR_FRACTION);
this->cfgSeverityFraction = config.getD(detector::SEVERITY_FRACTION);
setCfgWindowSize(_config.item<int64_t>(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove all cfg prefixes

detector::WINDOW_SIZE,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please start to move all statics to proper classes.
With proper names.

detector::DEFAULT_WINDOW_SIZE));

setCfgMaxCheckpoints(_config.item<int64_t>(
detector::MAX_CHECKPOINTS,
detector::DEFAULT_MAX_CHECKPOINTS));

setCfgQuroum(_config.item<double_t>(
detector::QUORUM,
detector::DEFAULT_QUORUM));

setCfgFractionalThreshold(_config.item<double_t>(
detector::FRACTIONAL_THRESHOLD,
detector::DEFAULT_FRACTIONAL_THRESHOLD));

setCfgNearFraction(_config.item<double_t>(
detector::NEAR_FRACTION,
detector::DEFAULT_NEAR_FRACTION));

setCfgSeverityFraction(_config.item<double_t>(
detector::SEVERITY_FRACTION,
detector::DEFAULT_SEVERITY_FRACTION));

this->recalculateParams();
}
Expand All @@ -131,6 +97,49 @@ class SignalDropAnalyzer : public SignalAnalyzer {
*/
void shiftBasePoints();

//! int64_t

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type information in comment is redundant.
Comments lies.
The same applies bellow.

//! How far in the past we look.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could precise it a little bit: "Sets the number of previous iterations the algorithm takes in account"

void setCfgWindowSize(int64_t cfgWindowSize) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/s/setCfgWindowSize/setWindowSize

The same applies below.

SignalDropAnalyzer::cfgWindowSize = cfgWindowSize;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is SignalDropAnalyzer::cfgWindowSize is static? Looks like ;)
You should rather do windowSize = _windowsSize;

}

//! int64_t
//! Maximum number of checkpoints we will have in our assurance detector.
//! Checkpoints are the reference assurance_test(base) points which we refer
//! to in the past when detecting drop or not.
//! It needs to be 0 < < WINDOW_SIZE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should check this condition in setter... but it makes unfortunate time coupling with setWindowSize function - could we make something with that?

We might need to create one setter for both those values.

void setCfgMaxCheckpoints(int64_t cfgMaxCheckpoints) {
SignalDropAnalyzer::cfgMaxCheckpoints = cfgMaxCheckpoints;
}

//! double_t
//! Fraction of checkpoints' votes that important decision needs to obtain.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"important decision needs to obtain" does not clears anything :)

void setCfgQuroum(double_t cfgQuroum) {
SignalDropAnalyzer::cfgQuroum = cfgQuroum;
}

//! double_t
//! Defines how much (relatively to base point) value must drop to trigger
//! contention.
//! Most signal_analyzer will use that.
void setCfgFractionalThreshold(double_t cfgFractionalThreshold) {
SignalDropAnalyzer::cfgFractionalThreshold = cfgFractionalThreshold;
}

//! double_t
//! You can adjust how big severity is created for a defined drop.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you should also write what 'severity' is.

//! if -1 then unknown severity will be reported.
void setCfgSeverityFraction(double_t cfgSeverityFraction) {
SignalDropAnalyzer::cfgSeverityFraction = cfgSeverityFraction;
}

//! double_t
//! Tolerance fraction of threshold if signal is accepted as returned to
//! previous state after drop.
void setCfgNearFraction(double_t cfgNearFraction) {
SignalDropAnalyzer::cfgNearFraction = cfgNearFraction;
}

protected:
std::list<double_t> window;
std::list<std::list<double_t>::iterator> basePoints;
Expand All @@ -142,16 +151,16 @@ class SignalDropAnalyzer : public SignalAnalyzer {
uint32_t quorumNum;

// cfg parameters.
uint64_t cfgWindowSize;
uint64_t cfgMaxCheckpoints;
int64_t cfgWindowSize;
int64_t cfgMaxCheckpoints;
double_t cfgQuroum;
double_t cfgFractionalThreshold;
double_t cfgSeverityFraction;
double_t cfgNearFraction;

/**
* It is possible to dynamically change analyzer configuration.
*/
* It is possible to dynamically change analyzer configuration.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is rather not helpful.

*/
void recalculateParams();
};

Expand Down
31 changes: 9 additions & 22 deletions src/filters/too_low_usage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,6 @@
namespace mesos {
namespace serenity {

class TooLowUsageFilterConfig : public SerenityConfig {
public:
TooLowUsageFilterConfig() { }

explicit TooLowUsageFilterConfig(const SerenityConfig& customCfg) {
this->initDefaults();
this->applyConfig(customCfg);
}

void initDefaults() {
//! double_t
//! Minimal cpu usage
this->fields[too_low_usage::MINIMAL_CPU_USAGE] =
too_low_usage::DEFAULT_MINIMAL_CPU_USAGE;
}
};


/**
* Filter out PR executors with too low metrics.
* Currently we filter out when CPU Usage is below specified threshold.
Expand All @@ -44,11 +26,12 @@ class TooLowUsageFilter :

explicit TooLowUsageFilter(
Consumer<ResourceUsage>* _consumer,
SerenityConfig _conf,
const SerenityConfig& _conf,
const Tag& _tag = Tag(QOS_CONTROLLER, NAME))
: Producer<ResourceUsage>(_consumer), tag(_tag) {
SerenityConfig config = TooLowUsageFilterConfig(_conf);
this->cfgMinimalCpuUsage = config.getD(too_low_usage::MINIMAL_CPU_USAGE);
setCfgMinimalCpuUsage(_conf.item<double_t>(
too_low_usage::MINIMAL_CPU_USAGE,
too_low_usage::DEFAULT_MINIMAL_CPU_USAGE));
}

~TooLowUsageFilter();
Expand All @@ -57,7 +40,11 @@ class TooLowUsageFilter :

Try<Nothing> consume(const ResourceUsage& in);

public:
void setCfgMinimalCpuUsage(double_t cfgMinimalCpuUsage) {
TooLowUsageFilter::cfgMinimalCpuUsage = cfgMinimalCpuUsage;
}

protected:
const Tag tag;

double_t cfgMinimalCpuUsage;
Expand Down
Loading