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

Jump to bottom

osd,compressor: Expose compression algorithms via MOSDBoot. #20558

Merged
merged 1 commit into from Mar 27, 2018
Merged

osd,compressor: Expose compression algorithms via MOSDBoot. #20558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 15 additions & 31 deletions src/compressor/Compressor.cc
View file
Open in desktop
Expand Up @@ -14,6 +14,8 @@

#include <random>
#include <sstream>
#include <iterator>
#include <algorithm>

#include "CompressionPlugin.h"
#include "Compressor.h"
Expand All @@ -22,41 +24,23 @@
#include "common/debug.h"
#include "common/dout.h"

const char * Compressor::get_comp_alg_name(int a) {
switch (a) {
case COMP_ALG_NONE: return "none";
case COMP_ALG_SNAPPY: return "snappy";
case COMP_ALG_ZLIB: return "zlib";
case COMP_ALG_ZSTD: return "zstd";
#ifdef HAVE_LZ4
case COMP_ALG_LZ4: return "lz4";
#endif
#ifdef HAVE_BROTLI
case COMP_ALG_BROTLI: return "brotli";
#endif
default: return "???";
}
std::string Compressor::get_comp_alg_name(int a) {

auto p = std::find_if(std::cbegin(compression_algorithms), std::cend(compression_algorithms),
[a](const auto& kv) { return kv.second == a; });

if (std::cend(compression_algorithms) == p)
return "???"; // It would be nice to revise this...

return std::string { p->first };
}

boost::optional<Compressor::CompressionAlgorithm> Compressor::get_comp_alg_type(const std::string &s) {
if (s == "snappy")
return COMP_ALG_SNAPPY;
if (s == "zlib")
return COMP_ALG_ZLIB;
if (s == "zstd")
return COMP_ALG_ZSTD;
#ifdef HAVE_LZ4
if (s == "lz4")
return COMP_ALG_LZ4;
#endif
#ifdef HAVE_BROTLI
if (s == "brotli")
return COMP_ALG_BROTLI;
#endif
if (s == "" || s == "none")
return COMP_ALG_NONE;

return boost::optional<CompressionAlgorithm>();
if (auto pos = compression_algorithms.find(s.c_str()); std::end(compression_algorithms) != pos)
return pos->second;

return boost::optional<Compressor::CompressionAlgorithm> {};
}

const char *Compressor::get_comp_mode_name(int m) {
Expand Down
22 changes: 18 additions & 4 deletions src/compressor/Compressor.h
View file
Open in desktop
Expand Up @@ -15,11 +15,12 @@
#ifndef CEPH_COMPRESSOR_H
#define CEPH_COMPRESSOR_H


#include <map>
#include <memory>
#include <string>
#include <string_view>
#include <boost/optional.hpp>
#include "include/assert.h" // boost clobbers this
#include "include/assert.h" // boost clobbers this
#include "include/buffer.h"
#include "include/int_types.h"

Expand All @@ -40,8 +41,21 @@ class Compressor {
#ifdef HAVE_BROTLI
COMP_ALG_BROTLI = 5,
#endif
COMP_ALG_LAST //the last value for range checks
COMP_ALG_LAST //the last value for range checks
};

inline static const std::map<const std::string, const CompressionAlgorithm> compression_algorithms {
Copy link
Contributor

Choose a reason for hiding this comment

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

i opened #21801 to make this a constexpr initializer_list

Copy link
Contributor

Choose a reason for hiding this comment

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

this exposes compression_algorithms directly as an interface to determine which are enabled. consider extending the static functions to do this instead. for example:

  static constexpr std::optional<const char*> get_comp_alg_name(int a);
  static constexpr std::optional<CompressionAlgorithm> get_comp_alg_type(std::string_view s);

the caller can loop over [0,COMP_ALG_LAST) and call get_comp_alg_name(i) to include the names of each algorithm enabled. these functions can be constexpr thanks to initializer_list and the constexpr constructors of std::optional and std::string_view. std::find_if() isn't constexpr until c++20, but a ranged-for loop would work instead

(this would also address the comment return "???"; // It would be nice to revise this...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, 'return "???"' was what the prior code did, and frankly I'm not a fan of that. I decided to lift up only a single level of abstraction because I didn't know how much other code might be using it, but there's certainly no reason it can't be revisited! Thanks for your thoughtful comments.

{ "none", COMP_ALG_NONE },
{ "snappy", COMP_ALG_SNAPPY },
{ "zlib", COMP_ALG_ZLIB },
#ifdef HAVE_LZ4
{ "lz4", COMP_ALG_LZ4 },
#endif
#ifdef HAVE_BROTLI
{ "brotli", COMP_ALG_BROTLI },
#endif
};

// compression options
enum CompressionMode {
COMP_NONE, ///< compress never
Expand All @@ -50,7 +64,7 @@ class Compressor {
COMP_FORCE ///< compress always
};

static const char * get_comp_alg_name(int a);
static std::string get_comp_alg_name(int a);
static boost::optional<CompressionAlgorithm> get_comp_alg_type(const std::string &s);

static const char *get_comp_mode_name(int m);
Expand Down
40 changes: 36 additions & 4 deletions src/osd/OSD.cc
View file
Open in desktop
Expand Up @@ -12,14 +12,21 @@
* Foundation. See file COPYING.
*
*/

#include "acconfig.h"
#include <unistd.h>

#include <cerrno>
#include <cctype>
#include <fstream>
#include <iostream>
#include <errno.h>
#include <algorithm>

#include <experimental/iterator>

#include <unistd.h>

#include <sys/stat.h>
#include <signal.h>
#include <ctype.h>
#include <boost/scoped_ptr.hpp>

#ifdef HAVE_SYS_PARAM_H
Expand Down Expand Up @@ -47,6 +54,7 @@
#include "common/io_priority.h"
#include "common/pick_address.h"
#include "common/SubProcess.h"
#include "common/PluginRegistry.h"

#include "os/ObjectStore.h"
#ifdef HAVE_LIBFUSE
Expand All @@ -55,7 +63,6 @@

#include "PrimaryLogPG.h"


#include "msg/Messenger.h"
#include "msg/Message.h"

Expand Down Expand Up @@ -5536,6 +5543,27 @@ void OSD::_send_boot()
set_state(STATE_BOOTING);
}

std::string OSD::_collect_compression_algorithms()
{
using std::experimental::make_ostream_joiner;

const auto& compression_algorithms = Compressor::compression_algorithms;
const auto& plugin_registry = cct->get_plugin_registry()->plugins;

if (plugin_registry.empty())
return {};

ostringstream os;

copy_if(begin(compression_algorithms), end(compression_algorithms),
make_ostream_joiner(os, ", "),
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like this will print pairs instead of just the names

Copy link
Contributor

Choose a reason for hiding this comment

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

#21809

[&plugin_registry](const auto& algorithm) {
return plugin_registry.end() != plugin_registry.find(algorithm.first);
});

return os.str();
}

void OSD::_collect_metadata(map<string,string> *pm)
{
// config info
Expand Down Expand Up @@ -5566,6 +5594,10 @@ void OSD::_collect_metadata(map<string,string> *pm)
set<string> devnames;
store->get_devices(&devnames);
(*pm)["devices"] = stringify(devnames);

// Other information:
(*pm)["supported_compression_algorithms"] = _collect_compression_algorithms();

dout(10) << __func__ << " " << *pm << dendl;
}

Expand Down
1 change: 1 addition & 0 deletions src/osd/OSD.h
View file
Open in desktop
Expand Up @@ -1927,6 +1927,7 @@ class OSD : public Dispatcher,
void _preboot(epoch_t oldest, epoch_t newest);
void _send_boot();
void _collect_metadata(map<string,string> *pmeta);
std::string _collect_compression_algorithms();

void start_waiting_for_healthy();
bool _is_healthy();
Expand Down