You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by gr...@apache.org on 2018/08/28 03:18:19 UTC
[1/2] kudu git commit: util: add once class based on std::call_once
Repository: kudu
Updated Branches:
refs/heads/master bb78520a7 -> 3bb5f56cf
util: add once class based on std::call_once
The existing KuduOnceDynamic class only allows calling functions of type
Status(void). I intend on adding an argument to some of the existing the
Status(void) methods that get called using a KuduOnceDynamic. As such,
this patch adds a KuduOnceLambda class that accepts a lambda and
otherwise has the same semantics as KuduOnceDynamic.
I've replaced the KuduOnceDynamics that call methods that I intend on
addding arguments to with their corresponding KuduOnceLambdas.
Change-Id: Ide56057a800bf07923031df5a2a76a42f0b15358
Reviewed-on: http://gerrit.cloudera.org:8080/11302
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/54afea09
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/54afea09
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/54afea09
Branch: refs/heads/master
Commit: 54afea093dd2f82aea6cc289e4f5409972c487b3
Parents: bb78520
Author: Andrew Wong <aw...@cloudera.com>
Authored: Wed Aug 22 23:09:37 2018 -0700
Committer: Andrew Wong <aw...@cloudera.com>
Committed: Tue Aug 28 00:21:53 2018 +0000
----------------------------------------------------------------------
src/kudu/cfile/bloomfile.cc | 2 +-
src/kudu/cfile/bloomfile.h | 2 +-
src/kudu/cfile/cfile_reader.cc | 2 +-
src/kudu/cfile/cfile_reader.h | 2 +-
src/kudu/tablet/deltafile.cc | 2 +-
src/kudu/tablet/deltafile.h | 2 +-
src/kudu/util/once-test.cc | 57 ++++++++++++++++++++++++-------------
src/kudu/util/once.cc | 8 ++++++
src/kudu/util/once.h | 47 +++++++++++++++++++++++++++---
9 files changed, 94 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/54afea09/src/kudu/cfile/bloomfile.cc
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/bloomfile.cc b/src/kudu/cfile/bloomfile.cc
index b3628ee..0dbfa63 100644
--- a/src/kudu/cfile/bloomfile.cc
+++ b/src/kudu/cfile/bloomfile.cc
@@ -233,7 +233,7 @@ BloomFileReader::BloomFileReader(unique_ptr<CFileReader> reader,
}
Status BloomFileReader::Init() {
- return init_once_.Init(&BloomFileReader::InitOnce, this);
+ return init_once_.Init([this] { return InitOnce(); });
}
Status BloomFileReader::InitOnce() {
http://git-wip-us.apache.org/repos/asf/kudu/blob/54afea09/src/kudu/cfile/bloomfile.h
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/bloomfile.h b/src/kudu/cfile/bloomfile.h
index 6e7b280..e1553ec 100644
--- a/src/kudu/cfile/bloomfile.h
+++ b/src/kudu/cfile/bloomfile.h
@@ -149,7 +149,7 @@ class BloomFileReader {
std::unique_ptr<CFileReader> reader_;
- KuduOnceDynamic init_once_;
+ KuduOnceLambda init_once_;
ScopedTrackedConsumption mem_consumption_;
};
http://git-wip-us.apache.org/repos/asf/kudu/blob/54afea09/src/kudu/cfile/cfile_reader.cc
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/cfile_reader.cc b/src/kudu/cfile/cfile_reader.cc
index 3211751..e0d7688 100644
--- a/src/kudu/cfile/cfile_reader.cc
+++ b/src/kudu/cfile/cfile_reader.cc
@@ -192,7 +192,7 @@ Status CFileReader::InitOnce() {
}
Status CFileReader::Init() {
- RETURN_NOT_OK_PREPEND(init_once_.Init(&CFileReader::InitOnce, this),
+ RETURN_NOT_OK_PREPEND(init_once_.Init([this] { return InitOnce(); }),
Substitute("failed to init CFileReader for block $0",
block_id().ToString()));
return Status::OK();
http://git-wip-us.apache.org/repos/asf/kudu/blob/54afea09/src/kudu/cfile/cfile_reader.h
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/cfile_reader.h b/src/kudu/cfile/cfile_reader.h
index 58fb60f..7263d3f 100644
--- a/src/kudu/cfile/cfile_reader.h
+++ b/src/kudu/cfile/cfile_reader.h
@@ -214,7 +214,7 @@ class CFileReader {
const TypeInfo *type_info_;
const TypeEncodingInfo *type_encoding_info_;
- KuduOnceDynamic init_once_;
+ KuduOnceLambda init_once_;
ScopedTrackedConsumption mem_consumption_;
};
http://git-wip-us.apache.org/repos/asf/kudu/blob/54afea09/src/kudu/tablet/deltafile.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/deltafile.cc b/src/kudu/tablet/deltafile.cc
index 4cd832f..805483c 100644
--- a/src/kudu/tablet/deltafile.cc
+++ b/src/kudu/tablet/deltafile.cc
@@ -255,7 +255,7 @@ DeltaFileReader::DeltaFileReader(unique_ptr<CFileReader> cf_reader,
delta_type_(delta_type) {}
Status DeltaFileReader::Init() {
- return init_once_.Init(&DeltaFileReader::InitOnce, this);
+ return init_once_.Init([this] { return InitOnce(); });
}
Status DeltaFileReader::InitOnce() {
http://git-wip-us.apache.org/repos/asf/kudu/blob/54afea09/src/kudu/tablet/deltafile.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/deltafile.h b/src/kudu/tablet/deltafile.h
index e1e418f..a7203c3 100644
--- a/src/kudu/tablet/deltafile.h
+++ b/src/kudu/tablet/deltafile.h
@@ -219,7 +219,7 @@ class DeltaFileReader : public DeltaStore,
// The type of this delta, i.e. UNDO or REDO.
const DeltaType delta_type_;
- KuduOnceDynamic init_once_;
+ KuduOnceLambda init_once_;
};
// Iterator over the deltas contained in a delta file.
http://git-wip-us.apache.org/repos/asf/kudu/blob/54afea09/src/kudu/util/once-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/once-test.cc b/src/kudu/util/once-test.cc
index c0a79b1..58ebd2c 100644
--- a/src/kudu/util/once-test.cc
+++ b/src/kudu/util/once-test.cc
@@ -26,6 +26,7 @@
#include "kudu/util/once.h"
#include "kudu/util/status.h"
#include "kudu/util/test_macros.h"
+#include "kudu/util/test_util.h"
#include "kudu/util/thread.h"
using std::vector;
@@ -35,15 +36,14 @@ namespace kudu {
namespace {
+template<class KuduOnceType>
struct Thing {
explicit Thing(bool should_fail)
: should_fail_(should_fail),
value_(0) {
}
- Status Init() {
- return once_.Init(&Thing::InitOnce, this);
- }
+ Status Init();
Status InitOnce() {
if (should_fail_) {
@@ -55,14 +55,40 @@ struct Thing {
const bool should_fail_;
int value_;
- KuduOnceDynamic once_;
+ KuduOnceType once_;
};
-} // anonymous namespace
+template<>
+Status Thing<KuduOnceDynamic>::Init() {
+ return once_.Init(&Thing<KuduOnceDynamic>::InitOnce, this);
+}
+
+template<>
+Status Thing<KuduOnceLambda>::Init() {
+ return once_.Init([this] { return InitOnce(); });
+}
+
+template<class KuduOnceType>
+static void InitOrGetInitted(Thing<KuduOnceType>* t, int i) {
+ if (i % 2 == 0) {
+ LOG(INFO) << "Thread " << i << " initting";
+ t->Init();
+ } else {
+ LOG(INFO) << "Thread " << i << " value: " << t->once_.init_succeeded();
+ }
+}
-TEST(TestOnce, KuduOnceDynamicTest) {
+} // anonymous namespace
+
+typedef ::testing::Types<KuduOnceDynamic, KuduOnceLambda> KuduOnceTypes;
+TYPED_TEST_CASE(TestOnce, KuduOnceTypes);
+
+template<class KuduOnceType>
+class TestOnce : public KuduTest {};
+
+TYPED_TEST(TestOnce, KuduOnceTest) {
{
- Thing t(false);
+ Thing<TypeParam> t(false);
ASSERT_EQ(0, t.value_);
ASSERT_FALSE(t.once_.init_succeeded());
@@ -74,7 +100,7 @@ TEST(TestOnce, KuduOnceDynamicTest) {
}
{
- Thing t(true);
+ Thing<TypeParam> t(true);
for (int i = 0; i < 2; i++) {
ASSERT_TRUE(t.Init().IsIllegalState());
ASSERT_EQ(0, t.value_);
@@ -83,17 +109,8 @@ TEST(TestOnce, KuduOnceDynamicTest) {
}
}
-static void InitOrGetInitted(Thing* t, int i) {
- if (i % 2 == 0) {
- LOG(INFO) << "Thread " << i << " initting";
- t->Init();
- } else {
- LOG(INFO) << "Thread " << i << " value: " << t->once_.init_succeeded();
- }
-}
-
-TEST(TestOnce, KuduOnceDynamicThreadSafeTest) {
- Thing thing(false);
+TYPED_TEST(TestOnce, KuduOnceThreadSafeTest) {
+ Thing<TypeParam> thing(false);
// The threads will read and write to thing.once_.initted. If access to
// it is not synchronized, TSAN will flag the access as data races.
@@ -101,7 +118,7 @@ TEST(TestOnce, KuduOnceDynamicThreadSafeTest) {
for (int i = 0; i < 10; i++) {
scoped_refptr<Thread> t;
ASSERT_OK(Thread::Create("test", Substitute("thread $0", i),
- &InitOrGetInitted, &thing, i, &t));
+ &InitOrGetInitted<TypeParam>, &thing, i, &t));
threads.push_back(t);
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/54afea09/src/kudu/util/once.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/once.cc b/src/kudu/util/once.cc
index fada777..867c73e 100644
--- a/src/kudu/util/once.cc
+++ b/src/kudu/util/once.cc
@@ -29,4 +29,12 @@ size_t KuduOnceDynamic::memory_footprint_including_this() const {
return kudu_malloc_usable_size(this) + memory_footprint_excluding_this();
}
+size_t KuduOnceLambda::memory_footprint_excluding_this() const {
+ return status_.memory_footprint_excluding_this();
+}
+
+size_t KuduOnceLambda::memory_footprint_including_this() const {
+ return kudu_malloc_usable_size(this) + memory_footprint_excluding_this();
+}
+
} // namespace kudu
http://git-wip-us.apache.org/repos/asf/kudu/blob/54afea09/src/kudu/util/once.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/once.h b/src/kudu/util/once.h
index 0f43064..32c426c 100644
--- a/src/kudu/util/once.h
+++ b/src/kudu/util/once.h
@@ -14,11 +14,12 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-#ifndef KUDU_UTIL_ONCE_H
-#define KUDU_UTIL_ONCE_H
+#pragma once
#include <stddef.h>
+#include <mutex>
+
#include "kudu/gutil/once.h"
#include "kudu/gutil/port.h"
#include "kudu/util/atomic.h"
@@ -111,6 +112,44 @@ class KuduOnceDynamic {
Status status_;
};
-} // namespace kudu
+// Similar to the KuduOnceDynamic class, but accepts a lambda function.
+class KuduOnceLambda {
+ public:
+ KuduOnceLambda()
+ : init_succeeded_(false) {}
+
+ // If the underlying `once_flag` has yet to be invoked, invokes the provided
+ // lambda and stores its return value. Otherwise, returns the stored Status.
+ template<typename Fn>
+ Status Init(Fn fn) {
+ std::call_once(once_flag_, [this, fn] {
+ status_ = fn();
+ if (PREDICT_TRUE(status_.ok())) {
+ init_succeeded_.Store(true, kMemOrderRelease);
+ }
+ });
+ return status_;
+ }
+
+ // Similar to KuduOnceDynamic, kMemOrderAcquire here and kMemOrderRelease in
+ // Init(), taken together, mean that threads can safely synchronize on
+ // ini_succeeded_.
+ bool init_succeeded() const {
+ return init_succeeded_.Load(kMemOrderAcquire);
+ }
+
+ // Returns the memory usage of this object without the object itself. Should
+ // be used when embedded inside another object.
+ size_t memory_footprint_excluding_this() const;
-#endif
+ // Returns the memory usage of this object including the object itself.
+ // Should be used when allocated on the heap.
+ size_t memory_footprint_including_this() const;
+
+ private:
+ AtomicBool init_succeeded_;
+ std::once_flag once_flag_;
+ Status status_;
+};
+
+} // namespace kudu
[2/2] kudu git commit: [build] add -D{LEAK,UNDEFINED}_SANITIZER
Posted by gr...@apache.org.
[build] add -D{LEAK,UNDEFINED}_SANITIZER
To enable __lsan_default_suppressions() defined in
src/kudu/util/sanitizer_options.cc, it's necessary to have
LEAK_SANITIZER macro enabled during the compilation.
Also, defined UNDEFINED_SANITIZER macro to compile-in
__ubsan_default_options() as well.
This patch fixes negotiation-test failures in ASAN builds.
This is a follow-up for a8d088e11b8b901a91e4820dcfdca9537a1e801a.
Change-Id: Ie8caeaa080b93f67768d3d2251c7a366390a48f9
Reviewed-on: http://gerrit.cloudera.org:8080/11340
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Alexey Serbin <as...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/3bb5f56c
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/3bb5f56c
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/3bb5f56c
Branch: refs/heads/master
Commit: 3bb5f56cf0929fc80ad98672d15baf0263f84a18
Parents: 54afea0
Author: Alexey Serbin <as...@cloudera.com>
Authored: Mon Aug 27 18:20:09 2018 -0700
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Tue Aug 28 03:08:59 2018 +0000
----------------------------------------------------------------------
CMakeLists.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/3bb5f56c/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/CMakeLists.txt b/CMakeLists.txt
index bfdacd5..11b47b6 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -311,8 +311,8 @@ if ("${KUDU_USE_ASAN}" AND "${KUDU_USE_TSAN}")
message(SEND_ERROR "Can only enable one of ASAN or TSAN at a time")
endif()
-# Flag to enable clang address sanitizer
-# This will only build if clang or a recent enough gcc is the chosen compiler
+# Flag to enable clang address sanitizer (using it along with leak sanitizer).
+# This will only build if clang or a recent enough gcc is the chosen compiler.
if (${KUDU_USE_ASAN})
if(NOT (("${COMPILER_FAMILY}" STREQUAL "clang") OR
("${COMPILER_FAMILY}" STREQUAL "gcc" AND "${COMPILER_VERSION}" VERSION_GREATER "4.8")))
@@ -332,6 +332,7 @@ if (${KUDU_USE_ASAN})
endif()
endif()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -DADDRESS_SANITIZER")
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DLEAK_SANITIZER")
endif()
if (${KUDU_USE_XRAY})
@@ -355,6 +356,7 @@ if (${KUDU_USE_UBSAN})
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize-blacklist=${CMAKE_CURRENT_SOURCE_DIR}/build-support/ubsan-blacklist.txt")
# Stop execution after UB or overflow is detected.
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-sanitize-recover")
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DUNDEFINED_SANITIZER")
endif ()
# Flag to enable thread sanitizer (clang or gcc 4.8)