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/25 04:43:59 UTC
incubator-kudu git commit: KUDU-1297 - Fix RowSetTree sorting on mac
osx
Repository: incubator-kudu
Updated Branches:
refs/heads/master faeffdc9d -> 687fabf40
KUDU-1297 - Fix RowSetTree sorting on mac osx
We use a sorting function, RSEndpointBySliceCompare(), for the std::sort()
call in RowSetTree::Reset().
Sorting functions have a hard requirement that they should only return
true iif a < b and should return false in all other cases. Our function would
behave well in all cases except when a == b, in which case it might return
true or false depending on whether a.endpoint_ == START or a.endpoint_ == STOP.
Apparently when this requirement is not met, in some cases, segfaults ensue:
https://schneide.wordpress.com/2010/11/01/bug-hunting-fun-with-stdsort/
The fix is to return false if a == b. I've made sure that this fixes
the failing test (rowset_tree-test) in mac osx.
Change-Id: I315b4a71438cccb7bdc12e8929b064e9e5074f71
Reviewed-on: http://gerrit.cloudera.org:8080/1885
Tested-by: David Ribeiro Alves <da...@cloudera.com>
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/687fabf4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/687fabf4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/687fabf4
Branch: refs/heads/master
Commit: 687fabf40e9dd420bf7846a7f5e02a4ea6b62f53
Parents: faeffdc
Author: David Alves <da...@cloudera.com>
Authored: Fri Jan 22 19:45:14 2016 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Mon Jan 25 03:43:33 2016 +0000
----------------------------------------------------------------------
src/kudu/tablet/rowset_tree-test.cc | 2 +-
src/kudu/tablet/rowset_tree.cc | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/687fabf4/src/kudu/tablet/rowset_tree-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/rowset_tree-test.cc b/src/kudu/tablet/rowset_tree-test.cc
index 4b09edb..e3488be 100644
--- a/src/kudu/tablet/rowset_tree-test.cc
+++ b/src/kudu/tablet/rowset_tree-test.cc
@@ -140,7 +140,7 @@ TEST_F(TestRowSetTree, TestEndpointsConsistency) {
StringPrintf("%04d", 12000))));
// Make tree
RowSetTree tree;
- tree.Reset(vec);
+ ASSERT_OK(tree.Reset(vec));
// Keep track of "currently open" intervals defined by the endpoints
unordered_set<RowSet*> open;
// Keep track of all rowsets that have been visited
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/687fabf4/src/kudu/tablet/rowset_tree.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/rowset_tree.cc b/src/kudu/tablet/rowset_tree.cc
index e4c0cd6..69ca35a 100644
--- a/src/kudu/tablet/rowset_tree.cc
+++ b/src/kudu/tablet/rowset_tree.cc
@@ -44,7 +44,8 @@ bool RSEndpointBySliceCompare(const RowSetTree::RSEndpoint& a,
if (slice_cmp) return slice_cmp < 0;
ptrdiff_t rs_cmp = a.rowset_ - b.rowset_;
if (rs_cmp) return rs_cmp < 0;
- return b.endpoint_ == RowSetTree::STOP;
+ if (a.endpoint_ != b.endpoint_) return a.endpoint_ == RowSetTree::START;
+ return false;
}
} // anonymous namespace