Skip to content
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.

Sign up for GitHub

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

admin: add /runtime_modify endpoint and make runtime work without fs backing #2837

Merged
merged 11 commits into from
Apr 2, 2018
Merged

admin: add /runtime_modify endpoint and make runtime work without fs backing #2837

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ac67270
admin: add /runtime_modify endpoint and change runtime to work withou…
Mar 16, 2018
831493e
implement override layers
Mar 28, 2018
fdf40dc
merge
Mar 28, 2018
9c0c779
fix opt build
Mar 28, 2018
5952eb1
json output
Mar 30, 2018
c248042
address comments
Mar 30, 2018
f9635c5
update
Mar 30, 2018
eaf7a66
remove todo
Mar 30, 2018
fbb8717
fix integration test
Mar 31, 2018
4575302
Merge branch 'master' of https://github.com/envoyproxy/envoy into run…
Apr 2, 2018
2c4a1f2
address more comments
Apr 2, 2018
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
35 changes: 32 additions & 3 deletions include/envoy/runtime/runtime.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,25 @@ class Snapshot {
absl::optional<uint64_t> uint_value_;
};

/**
* A provider of runtime values. One or more of these compose the snapshot's source of values,
* where successive layers override the previous ones.
*/
class OverrideLayer {
public:
virtual ~OverrideLayer() {}
/**
* @return const std::unordered_map<std::string, Entry>& the values in this layer.
*/
virtual const std::unordered_map<std::string, Entry>& values() const PURE;
/**
* @return const std::string& a user-friendly alias for this layer, e.g. "admin" or "disk".
*/
virtual const std::string& name() const PURE;
};

typedef std::unique_ptr<const OverrideLayer> OverrideLayerConstPtr;

/**
* Test if a feature is enabled using the built in random generator. This is done by generating
* a random number in the range 0-99 and seeing if this number is < the value stored in the
Expand Down Expand Up @@ -116,10 +135,13 @@ class Snapshot {
virtual uint64_t getInteger(const std::string& key, uint64_t default_value) const PURE;

/**
* Fetch the raw runtime entries map. The map data is safe only for the lifetime of the Snapshot.
* @return const std::unordered_map<std::string, const Entry>& the raw map of loaded values.
* Fetch the OverrideLayers that provide values in this snapshot. Layers are ordered from bottom
* to top; for instance, the second layer's entries override the first layer's entries, and so on.
* Any layer can add a key in addition to overriding keys in layers below. The layer vector is
* safe only for the lifetime of the Snapshot.
* @return const std::vector<OverrideLayerConstPtr>& the raw map of loaded values.
*/
virtual const std::unordered_map<std::string, const Entry>& getAll() const PURE;
virtual const std::vector<OverrideLayerConstPtr>& getLayers() const PURE;
};

/**
Expand All @@ -135,6 +157,13 @@ class Loader {
* fetched again when needed.
*/
virtual Snapshot& snapshot() PURE;

/**
* Merge the given map of key-value pairs into the runtime's state. To remove a previous merge for
* a key, use an empty string as the value.
* @param values the values to merge
*/
virtual void mergeValues(const std::unordered_map<std::string, std::string>& values) PURE;
};

typedef std::unique_ptr<Loader> LoaderPtr;
Expand Down
173 changes: 123 additions & 50 deletions source/common/runtime/runtime_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,26 +146,26 @@ std::string RandomGeneratorImpl::uuid() {
return std::string(uuid, UUID_LENGTH);
}

SnapshotImpl::SnapshotImpl(const std::string& root_path, const std::string& override_path,
RuntimeStats& stats, RandomGenerator& generator,
Api::OsSysCalls& os_sys_calls)
: generator_(generator), os_sys_calls_(os_sys_calls) {
try {
walkDirectory(root_path, "");
if (Filesystem::directoryExists(override_path)) {
walkDirectory(override_path, "");
stats.override_dir_exists_.inc();
} else {
stats.override_dir_not_exists_.inc();
}
bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value,
uint64_t random_value, uint64_t num_buckets) const {
return random_value % num_buckets < std::min(getInteger(key, default_value), num_buckets);
}

stats.load_success_.inc();
} catch (EnvoyException& e) {
stats.load_error_.inc();
ENVOY_LOG(debug, "error creating runtime snapshot: {}", e.what());
bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value) const {
// Avoid PNRG if we know we don't need it.
uint64_t cutoff = std::min(getInteger(key, default_value), static_cast<uint64_t>(100));
if (cutoff == 0) {
return false;
} else if (cutoff == 100) {
return true;
} else {
return generator_.random() % 100 < cutoff;
}
}

stats.num_keys_.set(values_.size());
bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value,
uint64_t random_value) const {
return featureEnabled(key, default_value, random_value, 100);
}

const std::string& SnapshotImpl::get(const std::string& key) const {
Expand All @@ -186,11 +186,50 @@ uint64_t SnapshotImpl::getInteger(const std::string& key, uint64_t default_value
}
}

const std::unordered_map<std::string, const Snapshot::Entry>& SnapshotImpl::getAll() const {
return values_;
const std::vector<Snapshot::OverrideLayerConstPtr>& SnapshotImpl::getLayers() const {
return layers_;
}

void SnapshotImpl::walkDirectory(const std::string& path, const std::string& prefix) {
SnapshotImpl::SnapshotImpl(RandomGenerator& generator, RuntimeStats& stats,
std::vector<OverrideLayerConstPtr>&& layers)
: layers_{std::move(layers)}, generator_{generator} {
for (const auto& layer : layers_) {
for (const auto& kv : layer->values()) {
values_.erase(kv.first);
values_.emplace(kv.first, kv.second);
}
}
stats.num_keys_.set(values_.size());
}

Snapshot::Entry SnapshotImpl::createEntry(const std::string& value) {
Entry entry{value, absl::nullopt};
// As a perf optimization, attempt to convert the entry's string into an integer. If we don't
// succeed that's fine.
uint64_t converted;
if (StringUtil::atoul(entry.string_value_.c_str(), converted)) {
entry.uint_value_ = converted;
}
return entry;
}

void AdminLayer::mergeValues(const std::unordered_map<std::string, std::string>& values) {
for (const auto& kv : values) {
values_.erase(kv.first);
if (!kv.second.empty()) {
values_.emplace(kv.first, SnapshotImpl::createEntry(kv.second));
}
}
stats_.admin_overrides_active_.set(values_.empty() ? 0 : 1);
}

DiskLayer::DiskLayer(const std::string& name, const std::string& path,
Api::OsSysCalls& os_sys_calls)
: OverrideLayerImpl{name}, os_sys_calls_(os_sys_calls) {
walkDirectory(path, "");
}

void DiskLayer::walkDirectory(const std::string& path, const std::string& prefix) {
ENVOY_LOG(debug, "walking directory: {}", path);
Directory current_dir(path);
while (true) {
Expand Down Expand Up @@ -226,7 +265,7 @@ void SnapshotImpl::walkDirectory(const std::string& path, const std::string& pre
// for small files. Also, as noted elsewhere, none of this is non-blocking which could
// theoretically lead to issues.
ENVOY_LOG(debug, "reading file: {}", full_path);
Entry entry;
std::string value;

// Read the file and remove any comments. A comment is a line starting with a '#' character.
// Comments are useful for placeholder files with no value.
Expand All @@ -238,39 +277,65 @@ void SnapshotImpl::walkDirectory(const std::string& path, const std::string& pre
}
if (line == lines.back()) {
const absl::string_view trimmed = StringUtil::rtrim(line);
entry.string_value_.append(trimmed.data(), trimmed.size());
value.append(trimmed.data(), trimmed.size());
} else {
entry.string_value_.append(std::string{line} + "\n");
value.append(std::string{line} + "\n");
}
}

// As a perf optimization, attempt to convert the string into an integer. If we don't
// succeed that's fine.
uint64_t converted;
if (StringUtil::atoul(entry.string_value_.c_str(), converted)) {
entry.uint_value_ = converted;
}

// Separate erase/insert calls required due to the value type being constant; this prevents
// the use of the [] operator. Can leverage insert_or_assign in C++17 in the future.
values_.erase(full_prefix);
values_.insert({full_prefix, entry});
values_.insert({full_prefix, SnapshotImpl::createEntry(value)});
}
}
}

LoaderImpl::LoaderImpl(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator& tls,
const std::string& root_symlink_path, const std::string& subdir,
const std::string& override_dir, Stats::Store& store,
RandomGenerator& generator, Api::OsSysCallsPtr os_sys_calls)
: watcher_(dispatcher.createFilesystemWatcher()), tls_(tls.allocateSlot()),
generator_(generator), root_path_(root_symlink_path + "/" + subdir),
override_path_(root_symlink_path + "/" + override_dir), stats_(generateStats(store)),
LoaderImpl::LoaderImpl(RandomGenerator& generator, Stats::Store& store,
ThreadLocal::SlotAllocator& tls)
: LoaderImpl(DoNotLoadSnapshot{}, generator, store, tls) {
loadNewSnapshot();
}

LoaderImpl::LoaderImpl(DoNotLoadSnapshot /* unused */, RandomGenerator& generator,
Stats::Store& store, ThreadLocal::SlotAllocator& tls)
: generator_(generator), stats_(generateStats(store)), admin_layer_(stats_),
tls_(tls.allocateSlot()) {}

std::unique_ptr<SnapshotImpl> LoaderImpl::createNewSnapshot() {
std::vector<Snapshot::OverrideLayerConstPtr> layers;
layers.emplace_back(std::make_unique<const AdminLayer>(admin_layer_));
return std::make_unique<SnapshotImpl>(generator_, stats_, std::move(layers));
}

void LoaderImpl::loadNewSnapshot() {
ThreadLocal::ThreadLocalObjectSharedPtr ptr = createNewSnapshot();
tls_->set([ptr = std::move(ptr)](Event::Dispatcher&)->ThreadLocal::ThreadLocalObjectSharedPtr {
Copy link
Member

Choose a reason for hiding this comment

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

nit: You can just capture ptr directly without move. With C++14 you might even be able to just do ptr = createNewSnapshot()?

Copy link
Author

@jsedgwick jsedgwick Apr 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

std::move avoids a shared_ptr copy so I did this by instinct. And no, since, we need ptr to be the TLS shared_ptr, not a unique_ptr<Snapshot>, since the TLS system requires this lambda to be copyable. IMO this is the cleanest way to get the casts/copyability right.

*edit: to be clear, you can do this without the first line but it would make the capture clause really long and unwieldy

Copy link
Member

Choose a reason for hiding this comment

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

OK

return ptr;
});
}

Snapshot& LoaderImpl::snapshot() { return tls_->getTyped<Snapshot>(); }

void LoaderImpl::mergeValues(const std::unordered_map<std::string, std::string>& values) {
admin_layer_.mergeValues(values);
loadNewSnapshot();
}

DiskBackedLoaderImpl::DiskBackedLoaderImpl(Event::Dispatcher& dispatcher,
ThreadLocal::SlotAllocator& tls,
const std::string& root_symlink_path,
const std::string& subdir,
const std::string& override_dir, Stats::Store& store,
RandomGenerator& generator,
Api::OsSysCallsPtr os_sys_calls)
: LoaderImpl(DoNotLoadSnapshot{}, generator, store, tls),
watcher_(dispatcher.createFilesystemWatcher()), root_path_(root_symlink_path + "/" + subdir),
override_path_(root_symlink_path + "/" + override_dir),
os_sys_calls_(std::move(os_sys_calls)) {
watcher_->addWatch(root_symlink_path, Filesystem::Watcher::Events::MovedTo,
[this](uint32_t) -> void { onSymlinkSwap(); });
[this](uint32_t) -> void { loadNewSnapshot(); });

onSymlinkSwap();
loadNewSnapshot();
}

RuntimeStats LoaderImpl::generateStats(Stats::Store& store) {
Expand All @@ -280,16 +345,24 @@ RuntimeStats LoaderImpl::generateStats(Stats::Store& store) {
return stats;
}

void LoaderImpl::onSymlinkSwap() {
current_snapshot_.reset(
new SnapshotImpl(root_path_, override_path_, stats_, generator_, *os_sys_calls_));
ThreadLocal::ThreadLocalObjectSharedPtr ptr_copy = current_snapshot_;
tls_->set([ptr_copy](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr {
return ptr_copy;
});
std::unique_ptr<SnapshotImpl> DiskBackedLoaderImpl::createNewSnapshot() {
std::vector<Snapshot::OverrideLayerConstPtr> layers;
try {
layers.push_back(std::make_unique<DiskLayer>("root", root_path_, *os_sys_calls_));
if (Filesystem::directoryExists(override_path_)) {
layers.push_back(std::make_unique<DiskLayer>("override", override_path_, *os_sys_calls_));
stats_.override_dir_exists_.inc();
} else {
stats_.override_dir_not_exists_.inc();
}
} catch (EnvoyException& e) {
layers.clear();
stats_.load_error_.inc();
Copy link
Member

Choose a reason for hiding this comment

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

What was the behavior before when we had a load error. Did we have any runtime values at all? Just wondering if this is a change in behavior or not. It seems reasonable to have admin layer, or even root, if some layer doesn't load, but just want to be clear of what behavior was before and now.

Copy link
Author

@jsedgwick jsedgwick Mar 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Before this PR, no values at all. And yes, the admin layer will be put in place even if one or both of the disk layers doesn't load. Now that I think about it, should it be all or nothing for the two disk layers? What behavior do you think is best?

Copy link
Member

Choose a reason for hiding this comment

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

I think it might be simplest to just do all or nothing for all layers. If not that, I would definitely do all or nothing for the disk layers IMO. I'm fine either way, but I think I would avoid the incremental behavior we have now.

ENVOY_LOG(debug, "error loading runtime values from disk: {}", e.what());
}
layers.push_back(std::make_unique<AdminLayer>(admin_layer_));
return std::make_unique<SnapshotImpl>(generator_, stats_, std::move(layers));
}

Snapshot& LoaderImpl::snapshot() { return tls_->getTyped<Snapshot>(); }

} // namespace Runtime
} // namespace Envoy
Loading