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

common: introduce md_config_cacher_t #20320

Merged
merged 2 commits into from
Feb 12, 2018
Merged

common: introduce md_config_cacher_t #20320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8aa8028
common: introduce md_config_cacher_t.
rzarzynski Feb 1, 2018
248750f
osd: PrimaryLogPG::do_osd_ops() uses md_config_cacher_t.
rzarzynski Feb 5, 2018
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
59 changes: 59 additions & 0 deletions src/common/config_cacher.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
// vim: ts=8 sw=2 smarttab
/*
* Ceph - scalable distributed file system
*
* Copyright (C) 2004-2006 Sage Weil <sage@newdream.net>
*
* This is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License version 2.1, as published by the Free Software
* Foundation. See file COPYING.
*
*/

#ifndef CEPH_CONFIG_CACHER_H
#define CEPH_CONFIG_CACHER_H

#include "common/config_obs.h"
#include "common/config.h"

template <typename ValueT>
class md_config_cacher_t : public md_config_obs_t {
md_config_t& conf;
const char* const option_name;
std::atomic<ValueT> value_cache;
Copy link
Contributor

Choose a reason for hiding this comment

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

a bit surprising, if nothing constrains the type of ValueT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks there is a static_assert-capable way.


const char** get_tracked_conf_keys() const override {
const static char* keys[] = { option_name, nullptr };
return keys;
}

void handle_conf_change(const md_config_t* conf,
const std::set<std::string>& changed) override {
if (changed.count(option_name)) {
value_cache.store(conf->get_val<ValueT>(option_name));
}
}

public:
md_config_cacher_t(md_config_t& conf,
const char* const option_name)
: conf(conf),
option_name(option_name) {
conf.add_observer(this);
std::atomic_init(&value_cache,
conf.get_val<ValueT>(option_name));
}

~md_config_cacher_t() {
conf.remove_observer(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

object lifetimes are going to get tricky here, especially if we're declaring these as static

one option is to require md_config_cacher_t to hold a reference to CephContext. another is to skip this call to remove_observer() for static variables, as ~md_config_t() doesn't appear to care whether all observers were safely removed

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to be pedantic about lifecycle, because in many case we want ot ensure the cct gets destroy (e.g., library code) and using static would prevent that from ever happening. It shouldn't be hard to attach instances of this to a class that gets cleaned up before cct (i.e., something that owns a ref to the cct).

}

operator ValueT() const {
return value_cache.load();
}
};

#endif // CEPH_CONFIG_CACHER_H

2 changes: 2 additions & 0 deletions src/osd/OSD.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ OSDService::OSDService(OSD *osd) :
recovery_gen_wq("recovery_gen_wq", cct->_conf->osd_recovery_thread_timeout,
&osd->disk_tp),
class_handler(osd->class_handler),
osd_max_object_size(*cct->_conf, "osd_max_object_size"),
osd_skip_data_digest(*cct->_conf, "osd_skip_data_digest"),
pg_epoch_lock("OSDService::pg_epoch_lock"),
publish_lock("OSDService::publish_lock"),
pre_publish_lock("OSDService::pre_publish_lock"),
Expand Down
4 changes: 4 additions & 0 deletions src/osd/OSD.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "common/WorkQueue.h"
#include "common/AsyncReserver.h"
#include "common/ceph_context.h"
#include "common/config_cacher.h"
#include "common/zipkin_trace.h"

#include "mgr/MgrClient.h"
Expand Down Expand Up @@ -262,6 +263,9 @@ class OSDService {
GenContextWQ recovery_gen_wq;
ClassHandler *&class_handler;

md_config_cacher_t<uint64_t> osd_max_object_size;
md_config_cacher_t<bool> osd_skip_data_digest;

void enqueue_back(OpQueueItem&& qi);
void enqueue_front(OpQueueItem&& qi);

Expand Down
17 changes: 8 additions & 9 deletions src/osd/PrimaryLogPG.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5493,9 +5493,8 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
ObjectState& obs = ctx->new_obs;
object_info_t& oi = obs.oi;
const hobject_t& soid = oi.soid;
bool skip_data_digest = osd->store->has_builtin_csum() &&
cct->_conf->osd_skip_data_digest;
const uint64_t osd_max_object_size = cct->_conf->osd_max_object_size;
const bool skip_data_digest = osd->store->has_builtin_csum() &&
osd->osd_skip_data_digest;

PGTransaction* t = ctx->op_t.get();

Expand Down Expand Up @@ -5554,9 +5553,9 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
// munge ZERO -> TRUNCATE? (don't munge to DELETE or we risk hosing attributes)
if (op.op == CEPH_OSD_OP_ZERO &&
obs.exists &&
op.extent.offset < osd_max_object_size &&
op.extent.offset < osd->osd_max_object_size &&
op.extent.length >= 1 &&
op.extent.length <= osd_max_object_size &&
op.extent.length <= osd->osd_max_object_size &&
op.extent.offset + op.extent.length >= oi.size) {
if (op.extent.offset >= oi.size) {
// no-op
Expand Down Expand Up @@ -6256,7 +6255,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
}
}
result = check_offset_and_length(op.extent.offset, op.extent.length,
osd_max_object_size, get_dpp());
osd->osd_max_object_size, get_dpp());
if (result < 0)
break;

Expand Down Expand Up @@ -6302,7 +6301,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
break;
}
result = check_offset_and_length(0, op.extent.length,
osd_max_object_size, get_dpp());
osd->osd_max_object_size, get_dpp());
if (result < 0)
break;

Expand Down Expand Up @@ -6348,7 +6347,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
++ctx->num_write;
{ // zero
result = check_offset_and_length(op.extent.offset, op.extent.length,
osd_max_object_size, get_dpp());
osd->osd_max_object_size, get_dpp());
if (result < 0)
break;

Expand Down Expand Up @@ -6413,7 +6412,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
}

result = check_offset_and_length(op.extent.offset, op.extent.length,
osd_max_object_size, get_dpp());
osd->osd_max_object_size, get_dpp());
if (result < 0)
break;

Expand Down