From dec63eac6ef90fa2de3c8d08ec22a33495132cbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Mon, 29 Feb 2016 15:48:25 +0000 Subject: [PATCH 1/3] commitlog: add commitlog entry move constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Default move constructor and assignment didn't handle reference to mutation (_mutation) properly. Fixes #935. Signed-off-by: Paweł Dziepak Message-Id: <1456760905-23478-1-git-send-email-pdziepak@scylladb.com> --- db/commitlog/commitlog_entry.cc | 16 ++++++++++++++++ db/commitlog/commitlog_entry.hh | 4 ++++ 2 files changed, 20 insertions(+) diff --git a/db/commitlog/commitlog_entry.cc b/db/commitlog/commitlog_entry.cc index e2326dd9f1..4cd68a95f4 100644 --- a/db/commitlog/commitlog_entry.cc +++ b/db/commitlog/commitlog_entry.cc @@ -44,6 +44,22 @@ commitlog_entry::commitlog_entry(stdx::optional mapping, const f , _mutation(mutation) { } +commitlog_entry::commitlog_entry(commitlog_entry&& ce) + : _mapping(std::move(ce._mapping)) + , _mutation_storage(std::move(ce._mutation_storage)) + , _mutation(_mutation_storage ? *_mutation_storage : ce._mutation) +{ +} + +commitlog_entry& commitlog_entry::operator=(commitlog_entry&& ce) +{ + if (this != &ce) { + this->~commitlog_entry(); + new (this) commitlog_entry(std::move(ce)); + } + return *this; +} + commitlog_entry commitlog_entry_writer::get_entry() const { if (_with_schema) { return commitlog_entry(_schema->get_column_mapping(), _mutation); diff --git a/db/commitlog/commitlog_entry.hh b/db/commitlog/commitlog_entry.hh index 508f4ec3d5..51eaf482a9 100644 --- a/db/commitlog/commitlog_entry.hh +++ b/db/commitlog/commitlog_entry.hh @@ -35,6 +35,10 @@ class commitlog_entry { public: commitlog_entry(stdx::optional mapping, frozen_mutation&& mutation); commitlog_entry(stdx::optional mapping, const frozen_mutation& mutation); + commitlog_entry(commitlog_entry&&); + commitlog_entry(const commitlog_entry&) = delete; + commitlog_entry& operator=(commitlog_entry&&); + commitlog_entry& operator=(const commitlog_entry&) = delete; const stdx::optional& mapping() const { return _mapping; } const frozen_mutation& mutation() const { return _mutation; } }; From e194835d8a4e6a2126c599ed95e5b04114e5ce09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Mon, 29 Feb 2016 15:50:55 +0000 Subject: [PATCH 2/3] tests/idl: add test for stdx::optional<> serialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak Message-Id: <1456761055-23916-1-git-send-email-pdziepak@scylladb.com> --- idl/idl_test.idl.hh | 7 +++++- tests/idl_test.cc | 52 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/idl/idl_test.idl.hh b/idl/idl_test.idl.hh index 4d8239f8fe..4b25ac65cc 100644 --- a/idl/idl_test.idl.hh +++ b/idl/idl_test.idl.hh @@ -64,4 +64,9 @@ struct writable_variants stub [[writable]] { boost::variant first; boost::variant second; boost::variant third; -}; \ No newline at end of file +}; + +struct compound_with_optional { + std::experimental::optional first; + simple_compound second; +}; diff --git a/tests/idl_test.cc b/tests/idl_test.cc index 6af89c60c8..9d1ca8d9da 100644 --- a/tests/idl_test.cc +++ b/tests/idl_test.cc @@ -28,11 +28,14 @@ #include #include +#include #include "bytes.hh" #include "bytes_ostream.hh" #include "serializer.hh" +namespace stdx = std::experimental; + struct simple_compound { // TODO: change this to test for #905 uint32_t foo; @@ -48,6 +51,27 @@ std::ostream& operator<<(std::ostream& os, const simple_compound& sc) return os << " { foo: " << sc.foo << ", bar: " << sc.bar << " }"; } +struct compound_with_optional { + stdx::optional first; + simple_compound second; + + bool operator==(const compound_with_optional& other) const { + return first == other.first && second == other.second; + } +}; + +std::ostream& operator<<(std::ostream& os, const compound_with_optional& v) +{ + os << " { first: "; + if (v.first) { + os << *v.first; + } else { + os << ""; + } + os << ", second: " << v.second << " }"; + return os; +} + struct wrapped_vector { std::vector vector; @@ -238,4 +262,32 @@ BOOST_AUTO_TEST_CASE(test_variant) auto v3 = wv_view.third(); auto&& compound2 = boost::apply_visitor(expect_writable_compound(), v3); BOOST_REQUIRE_EQUAL(compound2, sc2); +} + +BOOST_AUTO_TEST_CASE(test_compound_with_optional) +{ + simple_compound foo = { 0xdeadbeef, 0xbadc0ffe }; + simple_compound bar = { 0x12345678, 0x87654321 }; + + compound_with_optional one = { foo, bar }; + + bytes_ostream buf1; + ser::serialize(buf1, one); + BOOST_REQUIRE_EQUAL(buf1.size(), 29); + + auto bv1 = buf1.linearize(); + seastar::simple_input_stream in1(reinterpret_cast(bv1.data()), bv1.size()); + auto deser_one = ser::deserialize(in1, boost::type()); + BOOST_REQUIRE_EQUAL(one, deser_one); + + compound_with_optional two = { {}, foo }; + + bytes_ostream buf2; + ser::serialize(buf2, two); + BOOST_REQUIRE_EQUAL(buf2.size(), 17); + + auto bv2 = buf2.linearize(); + seastar::simple_input_stream in2(reinterpret_cast(bv2.data()), bv2.size()); + auto deser_two = ser::deserialize(in2, boost::type()); + BOOST_REQUIRE_EQUAL(two, deser_two); } \ No newline at end of file From 34ed930aa4d1b5aeb7f89939e79d2a671b938b2d Mon Sep 17 00:00:00 2001 From: "Raphael S. Carvalho" Date: Mon, 29 Feb 2016 13:15:14 -0300 Subject: [PATCH 3/3] sstables: fix lack of accuracy in disk usage report To report disk usage, scylla was only taking into account size of sstable data component. Other components such as index and filter may be relatively big too. Therefore, 'nodetool status' would report an innacurate disk usage. That can be fixed by taking into account size of all sstable components. Fixes #943. Signed-off-by: Raphael S. Carvalho Message-Id: <08453585223570006ac4d25fe5fb909ad6c140a5.1456762244.git.raphaelsc@scylladb.com> --- database.cc | 8 ++++---- database.hh | 2 +- sstables/sstables.cc | 22 +++++++++++----------- sstables/sstables.hh | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/database.cc b/database.cc index 139fab87f6..226f0582eb 100644 --- a/database.cc +++ b/database.cc @@ -516,9 +516,9 @@ future column_family::probe_file(sstring sstdir, sst }); } -void column_family::update_stats_for_new_sstable(uint64_t new_sstable_data_size) { - _stats.live_disk_space_used += new_sstable_data_size; - _stats.total_disk_space_used += new_sstable_data_size; +void column_family::update_stats_for_new_sstable(uint64_t disk_space_used_by_sstable) { + _stats.live_disk_space_used += disk_space_used_by_sstable; + _stats.total_disk_space_used += disk_space_used_by_sstable; _stats.live_sstable_count++; } @@ -530,7 +530,7 @@ void column_family::add_sstable(lw_shared_ptr sstable) { auto generation = sstable->generation(); // allow in-progress reads to continue using old list _sstables = make_lw_shared(*_sstables); - update_stats_for_new_sstable(sstable->data_size()); + update_stats_for_new_sstable(sstable->bytes_on_disk()); _sstables->emplace(generation, std::move(sstable)); } diff --git a/database.hh b/database.hh index c96cc59ed5..1224d49696 100644 --- a/database.hh +++ b/database.hh @@ -172,7 +172,7 @@ private: class memtable_flush_queue; std::unique_ptr _flush_queue; private: - void update_stats_for_new_sstable(uint64_t new_sstable_data_size); + void update_stats_for_new_sstable(uint64_t disk_space_used_by_sstable); void add_sstable(sstables::sstable&& sstable); void add_sstable(lw_shared_ptr sstable); void add_memtable(); diff --git a/sstables/sstables.cc b/sstables/sstables.cc index 3eefc25bcf..1fa3475603 100644 --- a/sstables/sstables.cc +++ b/sstables/sstables.cc @@ -931,6 +931,14 @@ future<> sstable::open_data() { return _index_file.size().then([this] (auto size) { _index_file_size = size; }); + }).then([this] { + // Get disk usage for this sstable (includes all components). + _bytes_on_disk = 0; + return do_for_each(_components, [this] (component_type c) { + return engine().file_size(this->filename(c)).then([this] (uint64_t bytes) { + _bytes_on_disk += bytes; + }); + }); }); }); @@ -1446,17 +1454,9 @@ uint64_t sstable::data_size() const { return _data_file_size; } -future sstable::bytes_on_disk() { - if (_bytes_on_disk) { - return make_ready_future(_bytes_on_disk); - } - return do_for_each(_components, [this] (component_type c) { - return engine().file_size(filename(c)).then([this] (uint64_t bytes) { - _bytes_on_disk += bytes; - }); - }).then([this] { - return make_ready_future(_bytes_on_disk); - }); +uint64_t sstable::bytes_on_disk() { + assert(_bytes_on_disk > 0); + return _bytes_on_disk; } const bool sstable::has_component(component_type f) const { diff --git a/sstables/sstables.hh b/sstables/sstables.hh index 760e729388..d42bba470c 100644 --- a/sstables/sstables.hh +++ b/sstables/sstables.hh @@ -275,7 +275,7 @@ public: } // Returns the total bytes of all components. - future bytes_on_disk(); + uint64_t bytes_on_disk(); partition_key get_first_partition_key(const schema& s) const; partition_key get_last_partition_key(const schema& s) const;