You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/01/21 22:54:57 UTC
[1/2] incubator-kudu git commit: README: update eclipse information
Repository: incubator-kudu
Updated Branches:
refs/heads/master 4d8fbe7ee -> ed01385a7
README: update eclipse information
Mostly due to C++11 nonsense. But after playing with the exclusion list some
more, I now have an indexed Kudu tree with the fewest false positives (i.e.
misplaced "red squigglies") I've seen yet.
Change-Id: I3f4fe4b64ddb57ceaa3daf6121b643c4ce12ee4b
Reviewed-on: http://gerrit.cloudera.org:8080/1839
Tested-by: Internal Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/0d8da085
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/0d8da085
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/0d8da085
Branch: refs/heads/master
Commit: 0d8da0850a53ed77e9ab848669224c3333c31635
Parents: 4d8fbe7
Author: Adar Dembo <ad...@cloudera.com>
Authored: Tue Jan 19 18:44:00 2016 -0800
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Thu Jan 21 19:30:57 2016 +0000
----------------------------------------------------------------------
README.adoc | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/0d8da085/README.adoc
----------------------------------------------------------------------
diff --git a/README.adoc b/README.adoc
index cfd73de..ae80060 100644
--- a/README.adoc
+++ b/README.adoc
@@ -370,30 +370,34 @@ Eclipse can be used as an IDE for Kudu. To generate Eclipse project files, run:
[source,bash]
----
$ rm -rf CMakeCache.txt CMakeFiles/
-$ cmake -G "Eclipse CDT4 - Unix Makefiles" ../..
+$ cmake -G "Eclipse CDT4 - Unix Makefiles" -DCMAKE_CXX_COMPILER_ARG1=-std=c++11 ../..
----
It's critical that _CMakeCache.txt_ be removed prior to running the generator,
otherwise the extra Eclipse generator logic (the CMakeFindEclipseCDT4.make module)
won't run and standard system includes will be missing from the generated project.
+Thanks to http://public.kitware.com/Bug/view.php?id=15102, the Eclipse generator
+ignores the -std=c++11 definition and we must add it manually on the command line
+via CMAKE_CXX_COMPILER_ARG1.
+
By default, the Eclipse CDT indexer will index everything under the _kudu/_
source tree. It tends to choke on certain complicated source files within
-_thirdparty/llvm_. In CDT 8.7.0, the indexer will generate so many errors that
-it'll exit early, causing many spurious syntax errors to be highlighted. In older
+_thirdparty_. In CDT 8.7.0, the indexer will generate so many errors that it'll
+exit early, causing many spurious syntax errors to be highlighted. In older
versions of CDT, it'll spin forever.
-Either way, _thirdparty/llvm_ must be excluded from indexing. To do this, right
-click on the project in the Project Explorer and select Properties. In the
-dialog box, select "C/C++ Project Paths", select the Source tab, highlight
+Either way, these complicated source files must be excluded from indexing. To do
+this, right click on the project in the Project Explorer and select Properties. In
+the dialog box, select "C/C++ Project Paths", select the Source tab, highlight
"Exclusion filter: (None)", and click "Edit...". In the new dialog box, click
-"Add...". Click "Browse..." and select _thirdparty/llvm-3.4.2.src_. Click OK all
-the way out and rebuild the project index by right clicking the project in the
-Project Explorer and selecting Index --> Rebuild.
+"Add Multiple...". Select every subdirectory inside _thirdparty_ except _installed_
+and _installed-deps_. Click OK all the way out and rebuild the project index by
+right clicking the project in the Project Explorer and selecting Index --> Rebuild.
With this exclusion, the only false positives (shown as "red squigglies") that
CDT presents appear to be in atomicops functions (`NoBarrier_CompareAndSwap` for
-example) and in VLOG() function calls.
+example).
Another Eclipse annoyance stems from the "[Targets]" linked resource that Eclipse
generates for each unit test. These are probably used for building within Eclipse,
[2/2] incubator-kudu git commit: KUDU-598. Fix a race in concurrent
btree which could segfault
Posted by to...@apache.org.
KUDU-598. Fix a race in concurrent btree which could segfault
This adds a regression test and fix for KUDU-598, which caused
an occasional segfault during ConcurrentBTreeTest.TestConcurrentInsert.
The issue was that, while inserting a new element into a leaf node,
we'd briefly end up with a NULL (or uninitialized) value pointer
associated with a valid key. Because the size of the value is
stored at the pointed-to location (rather than with the pointer)
we accidentally derefenced it before checking that our read of the
leaf node was non-racy.
The test works by adding a new 'race point' in the insert code
path, and adding a new test which exercises of the race points.
With the new race test, we hit the SEGV almost every time. With
the patch, I was able to loop the new test 1000 times and the
old non-racy test 10000 times on GCE without failure.
Change-Id: I0c728a1d8014342bbc06b713a8ffee82ca4aa6bd
Reviewed-on: http://gerrit.cloudera.org:8080/1859
Tested-by: Internal Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/ed01385a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/ed01385a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/ed01385a
Branch: refs/heads/master
Commit: ed01385a7bbdc40161f93cd42da7f59301c2c06e
Parents: 0d8da08
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Jan 21 09:29:30 2016 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Thu Jan 21 20:42:16 2016 +0000
----------------------------------------------------------------------
src/kudu/tablet/cbtree-test.cc | 26 ++++++++++++--
src/kudu/tablet/concurrent_btree.h | 63 ++++++++++++++++++++-------------
2 files changed, 61 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/ed01385a/src/kudu/tablet/cbtree-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/cbtree-test.cc b/src/kudu/tablet/cbtree-test.cc
index 44d9610..0f42e1b 100644
--- a/src/kudu/tablet/cbtree-test.cc
+++ b/src/kudu/tablet/cbtree-test.cc
@@ -69,6 +69,9 @@ class TestCBTree : public KuduTest {
InsertInLeaf(&lnode, &arena, key, val));
}
+ template<class Traits>
+ void DoTestConcurrentInsert();
+
};
// Ensure that the template magic to make the nodes sized
@@ -155,6 +158,13 @@ struct SmallFanoutTraits : public BTreeTraits {
static const size_t leaf_node_size = 92;
};
+// Enables yield() calls at interesting points of the btree
+// implementation to ensure that we are still correct even
+// with adversarial scheduling.
+struct RacyTraits : public SmallFanoutTraits {
+ static const size_t debug_raciness = 100;
+};
+
void MakeKey(char *kbuf, size_t len, int i) {
snprintf(kbuf, len, "key_%d%d", i % 10, i / 10);
}
@@ -400,7 +410,17 @@ TEST_F(TestCBTree, TestVersionLockConcurrent) {
// Each thread inserts a number of elements and then verifies that it can
// read them back.
TEST_F(TestCBTree, TestConcurrentInsert) {
- gscoped_ptr<CBTree<SmallFanoutTraits> > tree;
+ DoTestConcurrentInsert<SmallFanoutTraits>();
+}
+
+// Same, but with a tree that tries to provoke race conditions.
+TEST_F(TestCBTree, TestRacyConcurrentInsert) {
+ DoTestConcurrentInsert<RacyTraits>();
+}
+
+template<class TraitsClass>
+void TestCBTree::DoTestConcurrentInsert() {
+ gscoped_ptr<CBTree<TraitsClass> > tree;
int num_threads = 16;
int ins_per_thread = 30;
@@ -417,7 +437,7 @@ TEST_F(TestCBTree, TestConcurrentInsert) {
for (int i = 0; i < num_threads; i++) {
threads.push_back(new boost::thread(
- InsertAndVerify<SmallFanoutTraits>,
+ InsertAndVerify<TraitsClass>,
&go_barrier,
&done_barrier,
&tree,
@@ -432,7 +452,7 @@ TEST_F(TestCBTree, TestConcurrentInsert) {
// on areas of the key space diminishes.
for (int trial = 0; trial < n_trials; trial++) {
- tree.reset(new CBTree<SmallFanoutTraits>());
+ tree.reset(new CBTree<TraitsClass>());
go_barrier.wait();
done_barrier.wait();
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/ed01385a/src/kudu/tablet/concurrent_btree.h
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/concurrent_btree.h b/src/kudu/tablet/concurrent_btree.h
index f867284..916b833 100644
--- a/src/kudu/tablet/concurrent_btree.h
+++ b/src/kudu/tablet/concurrent_btree.h
@@ -92,6 +92,18 @@ inline void PrefetchMemory(const T *addr) {
}
}
+// Utility function that, when Traits::debug_raciness is non-zero
+// (i.e only in debug code), will spin for some amount of time
+// related to that setting.
+// This can be used when trying to debug race conditions, but
+// will compile away in production code.
+template<class Traits>
+void DebugRacyPoint() {
+ if (Traits::debug_raciness > 0) {
+ boost::detail::yield(Traits::debug_raciness);
+ }
+}
+
template<class Traits> class NodeBase;
template<class Traits> class InternalNode;
template<class Traits> class LeafNode;
@@ -702,9 +714,10 @@ class LeafNode : public NodeBase<Traits> {
this->SetInserting();
// The following inserts should always succeed because we
- // verified space_available above
+ // verified that there is space available above.
num_entries_++;
InsertInSliceArray(keys_, num_entries_, key, idx, arena);
+ DebugRacyPoint<Traits>();
InsertInSliceArray(vals_, num_entries_, val, idx, arena);
return INSERT_SUCCESS;
@@ -739,12 +752,12 @@ class LeafNode : public NodeBase<Traits> {
//
// NOTE: the value slice may include an *invalid pointer*, not
// just invalid data, so any readers should check for conflicts
- // before accessing any referred-to data in the value slice.
+ // before accessing the value slice.
// The key, on the other hand, will always be a valid pointer, but
// may be invalid data.
- void Get(size_t idx, Slice *k, Slice *v) const {
+ void Get(size_t idx, Slice *k, ValueSlice *v) const {
*k = keys_[idx].as_slice();
- *v = vals_[idx].as_slice();
+ *v = vals_[idx];
}
// Truncates the node, removing entries from the right to reduce
@@ -855,10 +868,11 @@ class PreparedMutation {
// has the same size as the original data.
Slice current_mutable_value() {
CHECK(prepared());
- Slice k, v;
+ Slice k;
+ ValueSlice v;
leaf_->Get(idx_, &k, &v);
leaf_->SetInserting();
- return v;
+ return v.as_slice();
}
// Accessors
@@ -990,7 +1004,8 @@ class CBTree {
retry_in_leaf:
{
GetResult ret;
- Slice key_in_node, val_in_node;
+ Slice key_in_node;
+ ValueSlice val_in_node;
bool exact;
size_t idx = leaf->Find(key, &exact);
DebugRacyPoint();
@@ -999,13 +1014,7 @@ class CBTree {
ret = GET_NOT_FOUND;
} else {
leaf->Get(idx, &key_in_node, &val_in_node);
- *buf_len = val_in_node.size();
-
- if (PREDICT_FALSE(val_in_node.size() > in_buf_len)) {
- ret = GET_TOO_BIG;
- } else {
- ret = GET_SUCCESS;
- }
+ ret = GET_SUCCESS;
}
// Got some kind of result, but may be based on racy data.
@@ -1017,8 +1026,18 @@ class CBTree {
version = new_version;
goto retry_in_leaf;
}
+
+ // If we found a matching key earlier, and the read of the node
+ // wasn't racy, we can safely work with the ValueSlice.
if (ret == GET_SUCCESS) {
- memcpy(buf, val_in_node.data(), val_in_node.size());
+ Slice val = val_in_node.as_slice();
+ *buf_len = val.size();
+
+ if (PREDICT_FALSE(val.size() > in_buf_len)) {
+ ret = GET_TOO_BIG;
+ } else {
+ memcpy(buf, val.data(), val.size());
+ }
}
return ret;
}
@@ -1179,16 +1198,8 @@ class CBTree {
return node.leaf_node_ptr();
}
-
- // Utility function that, when Traits::debug_raciness is non-zero
- // (i.e only in debug code), will spin for some amount of time
- // related to that setting.
- // This can be used when trying to debug race conditions, but
- // will compile away in production code.
void DebugRacyPoint() const {
- if (Traits::debug_raciness > 0) {
- boost::detail::yield(Traits::debug_raciness);
- }
+ btree::DebugRacyPoint<Traits>();
}
// Dump the tree.
@@ -1616,7 +1627,9 @@ class CBTreeIterator {
void GetCurrentEntry(Slice *key, Slice *val) const {
DCHECK(seeked_);
- leaf_to_scan_->Get(idx_in_leaf_, key, val);
+ ValueSlice val_slice;
+ leaf_to_scan_->Get(idx_in_leaf_, key, &val_slice);
+ *val = val_slice.as_slice();
}
Slice GetCurrentKey() const {