You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2017/06/09 20:28:48 UTC
[1/4] kudu git commit: Fix various use-after-move errors
Repository: kudu
Updated Branches:
refs/heads/master 6dade2aa0 -> 8d2ca982f
Fix various use-after-move errors
This fixes all of the cases where clang-tidy detects
'misc-use-after-move'. This is necessary before enabling protobuf move
constructors to avoid test failures.
Change-Id: I2895a3c334ee1f1e6c9d82d587973adf66ce3738
Reviewed-on: http://gerrit.cloudera.org:8080/7130
Reviewed-by: Dan Burkert <da...@apache.org>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/f1e75b72
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/f1e75b72
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/f1e75b72
Branch: refs/heads/master
Commit: f1e75b728e351d73b5439a8a46eec59977588dc1
Parents: 6dade2a
Author: Todd Lipcon <to...@cloudera.com>
Authored: Thu Jun 8 18:59:38 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Jun 9 20:27:10 2017 +0000
----------------------------------------------------------------------
build-support/lint.sh | 2 +-
src/kudu/client/client.cc | 6 ++++--
src/kudu/client/scan_token-test.cc | 2 +-
src/kudu/integration-tests/external_mini_cluster-test.cc | 2 +-
src/kudu/master/catalog_manager.cc | 7 ++++---
src/kudu/rpc/client_negotiation.cc | 2 --
src/kudu/tablet/lock_manager-test.cc | 2 +-
src/kudu/tablet/tablet_bootstrap-test.cc | 2 +-
src/kudu/util/status-test.cc | 10 +++++-----
9 files changed, 18 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/f1e75b72/build-support/lint.sh
----------------------------------------------------------------------
diff --git a/build-support/lint.sh b/build-support/lint.sh
index 61279bc..6cc2f74 100755
--- a/build-support/lint.sh
+++ b/build-support/lint.sh
@@ -51,7 +51,7 @@ cd $ROOT
$ROOT/thirdparty/installed/common/bin/cpplint.py \
--verbose=4 \
- --filter=-whitespace/comments,-readability/todo,-readability/inheritance,-build/header_guard,-build/include_order,-legal/copyright,-build/c++11 \
+ --filter=-whitespace/comments,-readability/todo,-readability/inheritance,-build/header_guard,-build/include_order,-legal/copyright,-build/c++11,-readability/nolint \
$FILES 2>&1 | grep -v 'Done processing' | tee $TMP
NUM_ERRORS=$(grep "Total errors found" $TMP | awk '{print $4}')
http://git-wip-us.apache.org/repos/asf/kudu/blob/f1e75b72/src/kudu/client/client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index b1bf502..3006f2f 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -989,15 +989,17 @@ KuduTableAlterer* KuduTableAlterer::RenameTo(const string& new_name) {
KuduColumnSpec* KuduTableAlterer::AddColumn(const string& name) {
Data::Step s = { AlterTableRequestPB::ADD_COLUMN,
new KuduColumnSpec(name), nullptr, nullptr };
+ auto* spec = s.spec;
data_->steps_.emplace_back(std::move(s));
- return s.spec;
+ return spec;
}
KuduColumnSpec* KuduTableAlterer::AlterColumn(const string& name) {
Data::Step s = { AlterTableRequestPB::ALTER_COLUMN,
new KuduColumnSpec(name), nullptr, nullptr };
+ auto* spec = s.spec;
data_->steps_.emplace_back(std::move(s));
- return s.spec;
+ return spec;
}
KuduTableAlterer* KuduTableAlterer::DropColumn(const string& name) {
http://git-wip-us.apache.org/repos/asf/kudu/blob/f1e75b72/src/kudu/client/scan_token-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/scan_token-test.cc b/src/kudu/client/scan_token-test.cc
index 521b752..58b62ef 100644
--- a/src/kudu/client/scan_token-test.cc
+++ b/src/kudu/client/scan_token-test.cc
@@ -191,7 +191,7 @@ TEST_F(ScanTokenTest, TestScanTokens) {
ASSERT_OK(builder.Build(&tokens));
ASSERT_EQ(1, tokens.size());
- ASSERT_EQ(1, CountRows(std::move(tokens)));
+ ASSERT_EQ(1, CountRows(tokens));
NO_FATALS(VerifyTabletInfo(tokens));
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/f1e75b72/src/kudu/integration-tests/external_mini_cluster-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster-test.cc b/src/kudu/integration-tests/external_mini_cluster-test.cc
index fb8eaed..79b2a7f 100644
--- a/src/kudu/integration-tests/external_mini_cluster-test.cc
+++ b/src/kudu/integration-tests/external_mini_cluster-test.cc
@@ -112,7 +112,7 @@ TEST_P(ExternalMiniClusterTest, TestBasicOperation) {
opts.num_masters = opts.master_rpc_ports.size();
opts.num_tablet_servers = 3;
- ExternalMiniCluster cluster(std::move(opts));
+ ExternalMiniCluster cluster(opts);
ASSERT_OK(cluster.Start());
// Verify each of the masters.
http://git-wip-us.apache.org/repos/asf/kudu/blob/f1e75b72/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index df7935e..8e19553 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -351,12 +351,13 @@ class TskEntryLoader : public TskEntryVisitor {
CHECK(tsk.has_expire_unix_epoch_seconds());
CHECK(tsk.has_rsa_key_der());
- // Expired entries are useful as well: they are needed for correct tracking
- // of TSK sequence numbers.
- entries_.emplace_back(std::move(tsk));
if (tsk.expire_unix_epoch_seconds() <= entry_expiration_seconds_) {
expired_entry_ids_.insert(entry_id);
}
+
+ // Expired entries are useful as well: they are needed for correct tracking
+ // of TSK sequence numbers.
+ entries_.emplace_back(std::move(tsk));
return Status::OK();
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/f1e75b72/src/kudu/rpc/client_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/client_negotiation.cc b/src/kudu/rpc/client_negotiation.cc
index 8ce5190..2b6b083 100644
--- a/src/kudu/rpc/client_negotiation.cc
+++ b/src/kudu/rpc/client_negotiation.cc
@@ -481,8 +481,6 @@ Status ClientNegotiation::HandleTlsHandshake(const NegotiatePB& response) {
RETURN_NOT_OK(s);
// TLS handshake is finished.
- DCHECK(token.empty());
-
if (ContainsKey(server_features_, TLS_AUTHENTICATION_ONLY) &&
ContainsKey(client_features_, TLS_AUTHENTICATION_ONLY)) {
TRACE("Negotiated auth-only $0 with cipher suite $1",
http://git-wip-us.apache.org/repos/asf/kudu/blob/f1e75b72/src/kudu/tablet/lock_manager-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/lock_manager-test.cc b/src/kudu/tablet/lock_manager-test.cc
index eccb67b..f599e2b 100644
--- a/src/kudu/tablet/lock_manager-test.cc
+++ b/src/kudu/tablet/lock_manager-test.cc
@@ -101,7 +101,7 @@ TEST_F(LockManagerTest, TestMoveLock) {
// Move it to a new instance.
ScopedRowLock moved_lock(std::move(row_lock));
ASSERT_TRUE(moved_lock.acquired());
- ASSERT_FALSE(row_lock.acquired());
+ ASSERT_FALSE(row_lock.acquired()); // NOLINT(misc-use-after-move)
}
class LmTestResource {
http://git-wip-us.apache.org/repos/asf/kudu/blob/f1e75b72/src/kudu/tablet/tablet_bootstrap-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_bootstrap-test.cc b/src/kudu/tablet/tablet_bootstrap-test.cc
index 0a0d5e9..4a3f735 100644
--- a/src/kudu/tablet/tablet_bootstrap-test.cc
+++ b/src/kudu/tablet/tablet_bootstrap-test.cc
@@ -589,7 +589,7 @@ TEST_F(BootstrapTest, TestConsensusOnlyOperationOutOfOrderTimestamp) {
AppendCommit(std::move(mutate_commit));
// ...and WRITE_OP...
- mutate_commit.reset(new consensus::CommitMsg);
+ mutate_commit = gscoped_ptr<consensus::CommitMsg>(new consensus::CommitMsg);
mutate_commit->set_op_type(consensus::WRITE_OP);
*mutate_commit->mutable_commited_op_id() = write_replicate->get()->id();
TxResultPB* result = mutate_commit->mutable_result();
http://git-wip-us.apache.org/repos/asf/kudu/blob/f1e75b72/src/kudu/util/status-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/status-test.cc b/src/kudu/util/status-test.cc
index ca33b89..d5aa130 100644
--- a/src/kudu/util/status-test.cc
+++ b/src/kudu/util/status-test.cc
@@ -50,7 +50,7 @@ TEST(StatusTest, TestMoveConstructor) {
{
Status src = Status::OK();
Status dst = std::move(src);
- ASSERT_OK(src);
+ ASSERT_OK(src); // NOLINT(misc-use-after-move)
ASSERT_OK(dst);
}
@@ -59,7 +59,7 @@ TEST(StatusTest, TestMoveConstructor) {
{
Status src = Status::NotFound("foo");
Status dst = std::move(src);
- ASSERT_OK(src);
+ ASSERT_OK(src); // NOLINT(misc-use-after-move)
ASSERT_EQ("Not found: foo", dst.ToString());
}
}
@@ -71,7 +71,7 @@ TEST(StatusTest, TestMoveAssignment) {
Status src = Status::OK();
Status dst = Status::NotFound("orig dst");
dst = std::move(src);
- ASSERT_OK(src);
+ ASSERT_OK(src); // NOLINT(misc-use-after-move)
ASSERT_OK(dst);
}
@@ -80,7 +80,7 @@ TEST(StatusTest, TestMoveAssignment) {
Status src = Status::NotFound("orig src");
Status dst = Status::NotFound("orig dst");
dst = std::move(src);
- ASSERT_OK(src);
+ ASSERT_OK(src); // NOLINT(misc-use-after-move)
ASSERT_EQ("Not found: orig src", dst.ToString());
}
@@ -89,7 +89,7 @@ TEST(StatusTest, TestMoveAssignment) {
Status src = Status::NotFound("orig src");
Status dst = Status::OK();
dst = std::move(src);
- ASSERT_OK(src);
+ ASSERT_OK(src); // NOLINT(misc-use-after-move)
ASSERT_EQ("Not found: orig src", dst.ToString());
}
}
[4/4] kudu git commit: Tweak build-and-test.sh to fix spark1 test
failures
Posted by da...@apache.org.
Tweak build-and-test.sh to fix spark1 test failures
Maven seems to be caching old artifacts which is leading to
hard-to-diagnose test failures in some branches. This commit changes the
spark1 test to do a full clean/rebuild instead of selectively removing
the kudu-spark target dir. I've also snuck in a change to add the -B
'batch' flag to maven invocations, which should help with the verbose
download progress log spam, as well as the -Dmaven.javadoc.skip flag in
order to speed up the builds, since javadocs aren't necessary.
Change-Id: I92fb0c05a9da65e1a41eb9017050cd8f957ce3a8
Reviewed-on: http://gerrit.cloudera.org:8080/7129
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/8d2ca982
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/8d2ca982
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/8d2ca982
Branch: refs/heads/master
Commit: 8d2ca982fe24170c6a8b899e4751a29f05d05f39
Parents: 3e24969
Author: Dan Burkert <da...@apache.org>
Authored: Thu Jun 8 15:06:22 2017 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Fri Jun 9 20:28:15 2017 +0000
----------------------------------------------------------------------
build-support/jenkins/build-and-test.sh | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/8d2ca982/build-support/jenkins/build-and-test.sh
----------------------------------------------------------------------
diff --git a/build-support/jenkins/build-and-test.sh b/build-support/jenkins/build-and-test.sh
index 5ebe1dd..c2c67d6 100755
--- a/build-support/jenkins/build-and-test.sh
+++ b/build-support/jenkins/build-and-test.sh
@@ -355,22 +355,19 @@ if [ "$BUILD_JAVA" == "1" ]; then
set -x
# Run the full Maven build (with Spark 2.x).
- if ! mvn $MVN_FLAGS \
- -Dsurefire.rerunFailingTestsCount=3 \
- -Dfailsafe.rerunFailingTestsCount=3 \
- clean verify ; then
+ MVN_FLAGS="$MVN_FLAGS -B"
+ MVN_FLAGS="$MVN_FLAGS -Dsurefire.rerunFailingTestsCount=3"
+ MVN_FLAGS="$MVN_FLAGS -Dfailsafe.rerunFailingTestsCount=3"
+ MVN_FLAGS="$MVN_FLAGS -Dmaven.javadoc.skip"
+ if ! mvn $MVN_FLAGS clean verify ; then
EXIT_STATUS=1
FAILURES="$FAILURES"$'Java build/test failed\n'
- else
- # If there were no failures, remove the Spark output and rerun the build,
- # this time just to test Spark 1.x with Scala 2.10.
- #
- # Note: this won't work if there are ever Spark integration tests!
- rm -rf kudu-spark/target/
- if ! mvn $MVN_FLAGS -Pspark_2.10 -Dtest="org.apache.kudu.spark.*.*" test; then
- EXIT_STATUS=1
- FAILURES="$FAILURES"$'spark build/test failed\n'
- fi
+
+ # If there are no failures, rerun the build with Spark 1.x with Scala 2.10.
+ # Note: this won't work if there are ever Spark integration tests!
+ elif ! mvn $MVN_FLAGS -Dtest="org.apache.kudu.spark.*.*" -Pspark_2.10 clean verify ; then
+ EXIT_STATUS=1
+ FAILURES="$FAILURES"$'Spark 1.x build/test failed\n'
fi
set +x
[3/4] kudu git commit: Fix all 'misc-string-compare' warnings from
clang-tidy
Posted by da...@apache.org.
Fix all 'misc-string-compare' warnings from clang-tidy
See https://clang.llvm.org/extra/clang-tidy/checks/misc-string-compare.html
for details
(auto-generated using clang-tidy --fix with only this check enabled)
Change-Id: I18e609172eda7793f79ed9bb8ddafdf3b85d5fb0
Reviewed-on: http://gerrit.cloudera.org:8080/7131
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/3e249697
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/3e249697
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/3e249697
Branch: refs/heads/master
Commit: 3e249697a506918319e89b1b1aecdb7cb5a634f5
Parents: 1632d16
Author: Todd Lipcon <to...@cloudera.com>
Authored: Thu Jun 8 19:06:46 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Jun 9 20:28:06 2017 +0000
----------------------------------------------------------------------
src/kudu/common/partition.cc | 4 ++--
src/kudu/util/compression/compression_codec.cc | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/3e249697/src/kudu/common/partition.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/partition.cc b/src/kudu/common/partition.cc
index c600b2a..bbb6343 100644
--- a/src/kudu/common/partition.cc
+++ b/src/kudu/common/partition.cc
@@ -68,8 +68,8 @@ Slice Partition::range_key(const string& partition_key) const {
bool Partition::Equals(const Partition& other) const {
if (this == &other) return true;
- if (partition_key_start().compare(other.partition_key_start()) != 0) return false;
- if (partition_key_end().compare(other.partition_key_end()) != 0) return false;
+ if (partition_key_start() != other.partition_key_start()) return false;
+ if (partition_key_end() != other.partition_key_end()) return false;
if (hash_buckets_ != other.hash_buckets_) return false;
return true;
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/3e249697/src/kudu/util/compression/compression_codec.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/compression/compression_codec.cc b/src/kudu/util/compression/compression_codec.cc
index ee774cd..4995023 100644
--- a/src/kudu/util/compression/compression_codec.cc
+++ b/src/kudu/util/compression/compression_codec.cc
@@ -266,13 +266,13 @@ CompressionType GetCompressionCodecType(const std::string& name) {
string uname;
ToUpperCase(name, &uname);
- if (uname.compare("SNAPPY") == 0)
+ if (uname == "SNAPPY")
return SNAPPY;
- if (uname.compare("LZ4") == 0)
+ if (uname == "LZ4")
return LZ4;
- if (uname.compare("ZLIB") == 0)
+ if (uname == "ZLIB")
return ZLIB;
- if (uname.compare("NONE") == 0)
+ if (uname == "NONE")
return NO_COMPRESSION;
LOG(WARNING) << "Unable to recognize the compression codec '" << name
[2/4] kudu git commit: Enable move constructors for protobufs
Posted by da...@apache.org.
Enable move constructors for protobufs
This enables an experimental option for protobuf to generate move
constructors for protos. This should make storing protobufs in
containers more performant, and also should allow us to avoid some
current patterns where we use ::Swap() instead of relying on more normal
looking C++11-style pass-by-value with std::move().
I didn't go through and update places to take advantage of this, except
for just trying it in one spot to make sure it compiles properly. I also
verified in the generated code that move constructors are being
generated.
Change-Id: I775e770799aec44cda79e641980e91259d19e650
Reviewed-on: http://gerrit.cloudera.org:8080/6900
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/1632d16a
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/1632d16a
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/1632d16a
Branch: refs/heads/master
Commit: 1632d16aa599334f175b91cebb01da3bf44839c3
Parents: f1e75b7
Author: Todd Lipcon <to...@apache.org>
Authored: Mon May 15 16:28:13 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Jun 9 20:27:54 2017 +0000
----------------------------------------------------------------------
src/kudu/rpc/client_negotiation.cc | 2 +-
thirdparty/build-definitions.sh | 8 +++++++-
thirdparty/download-thirdparty.sh | 5 +++++
3 files changed, 13 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/1632d16a/src/kudu/rpc/client_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/client_negotiation.cc b/src/kudu/rpc/client_negotiation.cc
index 2b6b083..c1e6581 100644
--- a/src/kudu/rpc/client_negotiation.cc
+++ b/src/kudu/rpc/client_negotiation.cc
@@ -524,7 +524,7 @@ Status ClientNegotiation::AuthenticateByToken(faststring* recv_buf,
// Send the token to the server.
NegotiatePB pb;
pb.set_step(NegotiatePB::TOKEN_EXCHANGE);
- pb.mutable_authn_token()->Swap(authn_token_.get_ptr());
+ *pb.mutable_authn_token() = std::move(*authn_token_);
RETURN_NOT_OK(SendNegotiatePB(pb));
pb.Clear();
http://git-wip-us.apache.org/repos/asf/kudu/blob/1632d16a/thirdparty/build-definitions.sh
----------------------------------------------------------------------
diff --git a/thirdparty/build-definitions.sh b/thirdparty/build-definitions.sh
index 4045cd0..fcbe9dc 100644
--- a/thirdparty/build-definitions.sh
+++ b/thirdparty/build-definitions.sh
@@ -336,10 +336,15 @@ build_gmock() {
build_protobuf() {
PROTOBUF_BDIR=$TP_BUILD_DIR/$PROTOBUF_NAME$MODE_SUFFIX
+ # Do a clean build if the patchlevel changed.
+ if [ "$(cd $PROTOBUF_BDIR && ls patchlevel* ||:)" != \
+ "$(cd $PROTOBUF_SOURCE && ls patchlevel* ||:)" ]; then
+ rm -Rf $PROTOBUF_BDIR
+ fi
mkdir -p $PROTOBUF_BDIR
pushd $PROTOBUF_BDIR
CFLAGS="$EXTRA_CFLAGS" \
- CXXFLAGS="$EXTRA_CXXFLAGS" \
+ CXXFLAGS="$EXTRA_CXXFLAGS -DPROTO_EXPERIMENTAL_ENABLE_MOVE" \
LDFLAGS="$EXTRA_LDFLAGS" \
LIBS="$EXTRA_LIBS" \
$PROTOBUF_SOURCE/configure \
@@ -349,6 +354,7 @@ build_protobuf() {
--prefix=$PREFIX
fixup_libtool
make -j$PARALLEL $EXTRA_MAKEFLAGS install
+ cp $PROTOBUF_SOURCE/patchlevel* .
popd
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/1632d16a/thirdparty/download-thirdparty.sh
----------------------------------------------------------------------
diff --git a/thirdparty/download-thirdparty.sh b/thirdparty/download-thirdparty.sh
index 68950f5..7cd5b25 100755
--- a/thirdparty/download-thirdparty.sh
+++ b/thirdparty/download-thirdparty.sh
@@ -139,9 +139,14 @@ if [ ! -d $GPERFTOOLS_SOURCE ]; then
echo
fi
+# We use a patchlevel=2 here to force a rebuild after changing CXXFLAGS.
+# The build itself isn't smart enough to know that this requires rebuilding.
+PROTOBUF_PATCHLEVEL=2
+delete_if_wrong_patchlevel $PROTOBUF_SOURCE $PROTOBUF_PATCHLEVEL
if [ ! -d $PROTOBUF_SOURCE ]; then
fetch_and_expand protobuf-${PROTOBUF_VERSION}.tar.gz
pushd $PROTOBUF_SOURCE
+ touch patchlevel-$PROTOBUF_PATCHLEVEL
autoreconf -fvi
popd
fi