http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/service/impala-server.h ---------------------------------------------------------------------- diff --git a/be/src/service/impala-server.h b/be/src/service/impala-server.h index c40fcb4..b884ea4 100644 --- a/be/src/service/impala-server.h +++ b/be/src/service/impala-server.h @@ -409,13 +409,13 @@ class ImpalaServer : public ImpalaServiceIf, public ImpalaHiveServer2ServiceIf, void LogQueryEvents(const QueryExecState& exec_state); /// Runs once every 5s to flush the profile log file to disk. - void LogFileFlushThread(); + [[noreturn]] void LogFileFlushThread(); /// Runs once every 5s to flush the audit log file to disk. - void AuditEventLoggerFlushThread(); + [[noreturn]] void AuditEventLoggerFlushThread(); /// Runs once every 5s to flush the lineage log file to disk. - void LineageLoggerFlushThread(); + [[noreturn]] void LineageLoggerFlushThread(); /// Copies a query's state into the query log. Called immediately prior to a /// QueryExecState's deletion. Also writes the query profile to the profile log on disk. @@ -516,7 +516,7 @@ class ImpalaServer : public ImpalaServiceIf, public ImpalaHiveServer2ServiceIf, void QueryHandleToTUniqueId(const beeswax::QueryHandle& handle, TUniqueId* query_id); /// Helper function to raise BeeswaxException - void RaiseBeeswaxException(const std::string& msg, const char* sql_state); + [[noreturn]] void RaiseBeeswaxException(const std::string& msg, const char* sql_state); /// Executes the fetch logic. Doesn't clean up the exec state if an error occurs. Status FetchInternal(const TUniqueId& query_id, bool start_over, @@ -598,12 +598,12 @@ class ImpalaServer : public ImpalaServiceIf, public ImpalaHiveServer2ServiceIf, /// sessions for their last-idle times. Those that have been idle for longer than /// their configured timeout values are 'expired': they will no longer accept queries /// and any running queries associated with those sessions are unregistered. - void ExpireSessions(); + [[noreturn]] void ExpireSessions(); /// Runs forever, walking queries_by_timestamp_ and expiring any queries that have been /// idle (i.e. no client input and no time spent processing locally) for /// FLAGS_idle_query_timeout seconds. - void ExpireQueries(); + [[noreturn]] void ExpireQueries(); /// Guards query_log_ and query_log_index_ boost::mutex query_log_lock_;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/service/query-exec-state.h ---------------------------------------------------------------------- diff --git a/be/src/service/query-exec-state.h b/be/src/service/query-exec-state.h index 78c859c..26dcf4d 100644 --- a/be/src/service/query-exec-state.h +++ b/be/src/service/query-exec-state.h @@ -170,7 +170,7 @@ class ImpalaServer::QueryExecState { } boost::mutex* lock() { return &lock_; } boost::mutex* fetch_rows_lock() { return &fetch_rows_lock_; } - const beeswax::QueryState::type query_state() const { return query_state_; } + beeswax::QueryState::type query_state() const { return query_state_; } const Status& query_status() const { return query_status_; } void set_result_metadata(const TResultSetMetadata& md) { result_metadata_ = md; } const RuntimeProfile& profile() const { return profile_; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/service/query-result-set.cc ---------------------------------------------------------------------- diff --git a/be/src/service/query-result-set.cc b/be/src/service/query-result-set.cc index 3b17af7..8d00af5 100644 --- a/be/src/service/query-result-set.cc +++ b/be/src/service/query-result-set.cc @@ -78,7 +78,7 @@ class HS2ColumnarResultSet : public QueryResultSet { public: HS2ColumnarResultSet(const TResultSetMetadata& metadata, TRowSet* rowset); - virtual ~HS2ColumnarResultSet(){}; + virtual ~HS2ColumnarResultSet() {} /// Add a row of expr values virtual Status AddOneRow(const vector<void*>& col_values, const vector<int>& scales); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/statestore/failure-detector.h ---------------------------------------------------------------------- diff --git a/be/src/statestore/failure-detector.h b/be/src/statestore/failure-detector.h index e71faff..c3f504e 100644 --- a/be/src/statestore/failure-detector.h +++ b/be/src/statestore/failure-detector.h @@ -77,7 +77,7 @@ class TimeoutFailureDetector : public FailureDetector { TimeoutFailureDetector(boost::posix_time::time_duration failure_timeout, boost::posix_time::time_duration suspect_timeout) : failure_timeout_(failure_timeout), - suspect_timeout_(suspect_timeout) { }; + suspect_timeout_(suspect_timeout) { } virtual PeerState UpdateHeartbeat(const std::string& peer, bool seen); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/statestore/statestore-subscriber.h ---------------------------------------------------------------------- diff --git a/be/src/statestore/statestore-subscriber.h b/be/src/statestore/statestore-subscriber.h index cb40080..1a24a76 100644 --- a/be/src/statestore/statestore-subscriber.h +++ b/be/src/statestore/statestore-subscriber.h @@ -263,7 +263,7 @@ class StatestoreSubscriber { /// During recovery mode, any public methods that are started will block on lock_, which /// is only released when recovery finishes. In practice, all registrations are made /// early in the life of an impalad before the statestore could be detected as failed. - void RecoveryModeChecker(); + [[noreturn]] void RecoveryModeChecker(); /// Creates a client of the remote statestore and sends a list of /// topics to register for. Returns OK unless there is some problem http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/statestore/statestore.cc ---------------------------------------------------------------------- diff --git a/be/src/statestore/statestore.cc b/be/src/statestore/statestore.cc index 5ef7cac..e63835f 100644 --- a/be/src/statestore/statestore.cc +++ b/be/src/statestore/statestore.cc @@ -199,7 +199,7 @@ void Statestore::Subscriber::AddTransientUpdate(const TopicId& topic_id, } } -const Statestore::TopicEntry::Version Statestore::Subscriber::LastTopicVersionProcessed( +Statestore::TopicEntry::Version Statestore::Subscriber::LastTopicVersionProcessed( const TopicId& topic_id) const { Topics::const_iterator itr = subscribed_topics_.find(topic_id); return itr == subscribed_topics_.end() ? @@ -561,7 +561,7 @@ void Statestore::GatherTopicUpdates(const Subscriber& subscriber, } } -const Statestore::TopicEntry::Version Statestore::GetMinSubscriberTopicVersion( +Statestore::TopicEntry::Version Statestore::GetMinSubscriberTopicVersion( const TopicId& topic_id, SubscriberId* subscriber_id) { TopicEntry::Version min_topic_version = numeric_limits<int64_t>::max(); bool found = false; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/statestore/statestore.h ---------------------------------------------------------------------- diff --git a/be/src/statestore/statestore.h b/be/src/statestore/statestore.h index 2bd5237..91adb8d 100644 --- a/be/src/statestore/statestore.h +++ b/be/src/statestore/statestore.h @@ -18,31 +18,31 @@ #ifndef STATESTORE_STATESTORE_H #define STATESTORE_STATESTORE_H -#include <stdint.h> +#include <cstdint> +#include <map> #include <string> #include <vector> -#include <map> -#include <boost/unordered_map.hpp> + #include <boost/scoped_ptr.hpp> #include <boost/thread/condition_variable.hpp> +#include <boost/unordered_map.hpp> #include <boost/uuid/uuid_generators.hpp> -#include "gen-cpp/Types_types.h" -#include "gen-cpp/StatestoreSubscriber.h" #include "gen-cpp/StatestoreService.h" -#include "util/metrics.h" -#include "util/collection-metrics.h" +#include "gen-cpp/StatestoreSubscriber.h" +#include "gen-cpp/Types_types.h" #include "rpc/thrift-client.h" -#include "util/thread-pool.h" -#include "util/webserver.h" #include "runtime/client-cache.h" #include "runtime/timestamp-value.h" #include "statestore/failure-detector.h" +#include "util/aligned-new.h" +#include "util/collection-metrics.h" +#include "util/metrics.h" +#include "util/thread-pool.h" +#include "util/webserver.h" namespace impala { -using namespace impala; - class Status; /// The Statestore is a soft-state key-value store that maintains a set of Topics, which @@ -79,7 +79,7 @@ class Status; /// processed. The statestore can use this information to send a delta of updates to a /// subscriber, rather than all items in the topic. For non-delta updates, the statestore /// will send an update that includes all values in the topic. -class Statestore { +class Statestore : public CacheLineAligned { public: /// A SubscriberId uniquely identifies a single subscriber, and is /// provided by the subscriber at registration time. @@ -251,6 +251,7 @@ class Statestore { /// Protects access to exit_flag_, but is used mostly to ensure visibility of updates /// between threads.. boost::mutex exit_flag_lock_; + bool exit_flag_; /// Controls access to topics_. Cannot take subscribers_lock_ after acquiring this lock. @@ -308,7 +309,7 @@ class Statestore { /// Returns the last version of the topic which this subscriber has successfully /// processed. Will never decrease. - const TopicEntry::Version LastTopicVersionProcessed(const TopicId& topic_id) const; + TopicEntry::Version LastTopicVersionProcessed(const TopicId& topic_id) const; /// Sets the subscriber's last processed version of the topic to the given value. This /// should only be set when once a subscriber has succesfully processed the given @@ -484,8 +485,8 @@ class Statestore { /// TODO: Update the min subscriber version only when a topic is updated, rather than /// each time a subscriber is updated. One way to do this would be to keep a priority /// queue in Topic of each subscriber's last processed version of the topic. - const TopicEntry::Version GetMinSubscriberTopicVersion(const TopicId& topic_id, - SubscriberId* subscriber_id = NULL); + TopicEntry::Version GetMinSubscriberTopicVersion( + const TopicId& topic_id, SubscriberId* subscriber_id = NULL); /// True if the shutdown flag has been set true, false otherwise. bool ShouldExit(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/testutil/test-udas.cc ---------------------------------------------------------------------- diff --git a/be/src/testutil/test-udas.cc b/be/src/testutil/test-udas.cc index 844bd33..dc295b5 100644 --- a/be/src/testutil/test-udas.cc +++ b/be/src/testutil/test-udas.cc @@ -84,7 +84,7 @@ void MemTestMerge(FunctionContext* context, const BigIntVal& src, BigIntVal* dst dst->val += src.val; } -const BigIntVal MemTestSerialize(FunctionContext* context, const BigIntVal& total) { +BigIntVal MemTestSerialize(FunctionContext* context, const BigIntVal& total) { if (total.is_null) return BigIntVal(0); context->Free(total.val); return total; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/testutil/test-udas.h ---------------------------------------------------------------------- diff --git a/be/src/testutil/test-udas.h b/be/src/testutil/test-udas.h index f0d796c..8cd38ec 100644 --- a/be/src/testutil/test-udas.h +++ b/be/src/testutil/test-udas.h @@ -25,7 +25,7 @@ using namespace impala_udf; void MemTestInit(FunctionContext*, BigIntVal* total); void MemTestUpdate(FunctionContext* context, const BigIntVal& bytes, BigIntVal* total); void MemTestMerge(FunctionContext* context, const BigIntVal& src, BigIntVal* dst); -const BigIntVal MemTestSerialize(FunctionContext* context, const BigIntVal& total); +BigIntVal MemTestSerialize(FunctionContext* context, const BigIntVal& total); BigIntVal MemTestFinalize(FunctionContext* context, const BigIntVal& total); #endif http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/transport/TSasl.h ---------------------------------------------------------------------- diff --git a/be/src/transport/TSasl.h b/be/src/transport/TSasl.h index c66fcf0..ca13f10 100644 --- a/be/src/transport/TSasl.h +++ b/be/src/transport/TSasl.h @@ -18,8 +18,8 @@ * under the License. */ -#ifndef _THRIFT_TRANSPORT_TSASL_H_ -#define _THRIFT_TRANSPORT_TSASL_H_ 1 +#ifndef IMPALA_TRANSPORT_TSASL_H +#define IMPALA_TRANSPORT_TSASL_H #include <string> #include <map> @@ -38,7 +38,10 @@ #include <thrift/transport/TTransportException.h> +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wheader-hygiene" using namespace apache::thrift::transport; +#pragma clang diagnostic pop namespace sasl { class SaslException : public TTransportException { @@ -186,4 +189,4 @@ class TSaslServer : public sasl::TSasl { bool serverStarted; }; } -#endif /* _THRIFT_TRANSPORT_TSALS_H_ */ +#endif /* IMPALA_TRANSPORT_TSASL_H */ http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/transport/TSaslClientTransport.h ---------------------------------------------------------------------- diff --git a/be/src/transport/TSaslClientTransport.h b/be/src/transport/TSaslClientTransport.h index 36dd65d..da708ae 100644 --- a/be/src/transport/TSaslClientTransport.h +++ b/be/src/transport/TSaslClientTransport.h @@ -18,8 +18,8 @@ * under the License. */ -#ifndef _THRIFT_TRANSPORT_TSSLCLIENTTRANSPORT_H_ -#define _THRIFT_TRANSPORT_TSSLCLIENTTRANSPORT_H_ 1 +#ifndef IMPALA_TRANSPORT_TSSLCLIENTTRANSPORT_H +#define IMPALA_TRANSPORT_TSSLCLIENTTRANSPORT_H #include <string> @@ -54,6 +54,4 @@ class TSaslClientTransport : public TSaslTransport { }}} // apache::thrift::transport -#endif // #ifndef _THRIFT_TRANSPORT_TSSLCLIENTTRANSPORT_H_ - - +#endif // #ifndef IMPALA_TRANSPORT_TSSLCLIENTTRANSPORT_H http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/transport/TSaslServerTransport.h ---------------------------------------------------------------------- diff --git a/be/src/transport/TSaslServerTransport.h b/be/src/transport/TSaslServerTransport.h index 28d17e6..c662771 100644 --- a/be/src/transport/TSaslServerTransport.h +++ b/be/src/transport/TSaslServerTransport.h @@ -18,8 +18,8 @@ * under the License. */ -#ifndef _THRIFT_TRANSPORT_TSASLSERVERTRANSPORT_H_ -#define _THRIFT_TRANSPORT_TSASLSERVERTRANSPORT_H_ 1 +#ifndef IMPALA_TRANSPORT_TSASLSERVERTRANSPORT_H +#define IMPALA_TRANSPORT_TSASLSERVERTRANSPORT_H #include <string> #include <pthread.h> @@ -186,4 +186,4 @@ class TSaslServerTransport : public TSaslTransport { }}} // apache::thrift::transport -#endif // #ifndef _THRIFT_TRANSPORT_TSSLSERVERTRANSPORT_H_ +#endif // #ifndef IMPALA_TRANSPORT_TSSLSERVERTRANSPORT_H http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/transport/TSaslTransport.h ---------------------------------------------------------------------- diff --git a/be/src/transport/TSaslTransport.h b/be/src/transport/TSaslTransport.h index 7dd0fb5..602f089 100644 --- a/be/src/transport/TSaslTransport.h +++ b/be/src/transport/TSaslTransport.h @@ -18,8 +18,8 @@ * under the License. */ -#ifndef _THRIFT_TRANSPORT_TSSLTRANSPORT_H_ -#define _THRIFT_TRANSPORT_TSSLTRANSPORT_H_ 1 +#ifndef IMPALA_TRANSPORT_TSSLTRANSPORT_H +#define IMPALA_TRANSPORT_TSSLTRANSPORT_H #include <string> @@ -220,4 +220,4 @@ class TSaslTransport : public TVirtualTransport<TSaslTransport> { }}} // apache::thrift::transport -#endif // #ifndef _THRIFT_TRANSPORT_TSSLTRANSPORT_H_ +#endif // #ifndef IMPALA_TRANSPORT_TSSLTRANSPORT_H http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf/uda-test-harness.h ---------------------------------------------------------------------- diff --git a/be/src/udf/uda-test-harness.h b/be/src/udf/uda-test-harness.h index b1e3847..7f871cc 100644 --- a/be/src/udf/uda-test-harness.h +++ b/be/src/udf/uda-test-harness.h @@ -41,12 +41,14 @@ enum UdaExecutionMode { template<typename RESULT, typename INTERMEDIATE> class UdaTestHarnessBase { public: + virtual ~UdaTestHarnessBase() = default; + typedef void (*InitFn)(FunctionContext* context, INTERMEDIATE* result); typedef void (*MergeFn)(FunctionContext* context, const INTERMEDIATE& src, INTERMEDIATE* dst); - typedef const INTERMEDIATE (*SerializeFn)(FunctionContext* context, + typedef INTERMEDIATE (*SerializeFn)(FunctionContext* context, const INTERMEDIATE& type); typedef RESULT (*FinalizeFn)(FunctionContext* context, const INTERMEDIATE& value); @@ -142,6 +144,8 @@ class UdaTestHarnessBase { template<typename RESULT, typename INTERMEDIATE, typename INPUT> class UdaTestHarness : public UdaTestHarnessBase<RESULT, INTERMEDIATE> { public: + virtual ~UdaTestHarness() = default; + typedef void (*UpdateFn)(FunctionContext* context, const INPUT& input, INTERMEDIATE* result); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf/uda-test.cc ---------------------------------------------------------------------- diff --git a/be/src/udf/uda-test.cc b/be/src/udf/uda-test.cc index 6f21699..3ca145f 100644 --- a/be/src/udf/uda-test.cc +++ b/be/src/udf/uda-test.cc @@ -116,7 +116,7 @@ void MinUpdate(FunctionContext* context, const StringVal& input, BufferVal* val) } // Serialize the state into the min string -const BufferVal MinSerialize(FunctionContext* context, const BufferVal& intermediate) { +BufferVal MinSerialize(FunctionContext* context, const BufferVal& intermediate) { MinState* state = reinterpret_cast<MinState*>(intermediate); if (state->value == NULL) return intermediate; // Hack to persist the intermediate state's value without leaking. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf/udf-ir.cc ---------------------------------------------------------------------- diff --git a/be/src/udf/udf-ir.cc b/be/src/udf/udf-ir.cc index aef835b..24773f0 100644 --- a/be/src/udf/udf-ir.cc +++ b/be/src/udf/udf-ir.cc @@ -43,10 +43,8 @@ void* FunctionContext::GetFunctionState(FunctionStateScope scope) const { switch (scope) { case THREAD_LOCAL: return impl_->thread_local_fn_state_; - break; case FRAGMENT_LOCAL: return impl_->fragment_local_fn_state_; - break; default: // TODO: signal error somehow return NULL; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf/udf.cc ---------------------------------------------------------------------- diff --git a/be/src/udf/udf.cc b/be/src/udf/udf.cc index 4ad8d8b..bfdbccc 100644 --- a/be/src/udf/udf.cc +++ b/be/src/udf/udf.cc @@ -31,7 +31,7 @@ // on libhdfs. #include "udf/udf-internal.h" -#if IMPALA_UDF_SDK_BUILD +#if defined(IMPALA_UDF_SDK_BUILD) && IMPALA_UDF_SDK_BUILD // For the SDK build, we are building the .lib that the developers would use to // write UDFs. They want to link against this to run their UDFs in a test environment. // Pulling in free-pool is very undesirable since it pulls in many other libraries. @@ -252,7 +252,7 @@ const char* FunctionContext::effective_user() const { FunctionContext::UniqueId FunctionContext::query_id() const { UniqueId id; -#if IMPALA_UDF_SDK_BUILD +#if defined(IMPALA_UDF_SDK_BUILD) && IMPALA_UDF_SDK_BUILD id.hi = id.lo = 0; #else id.hi = impl_->state_->query_id().hi; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf/udf.h ---------------------------------------------------------------------- diff --git a/be/src/udf/udf.h b/be/src/udf/udf.h index 5efbd84..e6ec07c 100644 --- a/be/src/udf/udf.h +++ b/be/src/udf/udf.h @@ -565,7 +565,7 @@ struct StringVal : public AnyVal { StringVal(uint8_t* ptr = NULL, int len = 0) : len(len), ptr(ptr) { assert(len >= 0); if (ptr == NULL) assert(len == 0); - }; + } /// Construct a StringVal from NULL-terminated c-string. Note: this does not make a /// copy of ptr so the underlying string must exist as long as this StringVal does. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/udf_samples/uda-sample.cc ---------------------------------------------------------------------- diff --git a/be/src/udf_samples/uda-sample.cc b/be/src/udf_samples/uda-sample.cc index 71764f3..23ce807 100644 --- a/be/src/udf_samples/uda-sample.cc +++ b/be/src/udf_samples/uda-sample.cc @@ -50,7 +50,7 @@ struct AvgStruct { }; void AvgInit(FunctionContext* context, BufferVal* val) { - assert(sizeof(AvgStruct) == 16); + static_assert(sizeof(AvgStruct) == 16, "AvgStruct is an unexpected size"); memset(*val, 0, sizeof(AvgStruct)); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/aligned-new.h ---------------------------------------------------------------------- diff --git a/be/src/util/aligned-new.h b/be/src/util/aligned-new.h new file mode 100644 index 0000000..3a4270c --- /dev/null +++ b/be/src/util/aligned-new.h @@ -0,0 +1,55 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#ifndef IMPALA_UTIL_ALIGNED_NEW_H_ +#define IMPALA_UTIL_ALIGNED_NEW_H_ + +#include <memory> + +#include "common/logging.h" + +namespace impala { + +// Objects that should be allocated, for performance or correctness reasons, at alignment +// greater than that promised by the global new (16) can inherit publicly from AlignedNew. +template <size_t ALIGNMENT> +struct alignas(ALIGNMENT) AlignedNew { + static_assert(ALIGNMENT > 0, "ALIGNMENT must be positive"); + static_assert((ALIGNMENT & (ALIGNMENT - 1)) == 0, "ALIGNMENT must be a power of 2"); + static_assert( + (ALIGNMENT % sizeof(void*)) == 0, "ALIGNMENT must be a multiple of sizeof(void *)"); + static void* operator new(std::size_t count) { return Allocate(count); } + static void* operator new[](std::size_t count) { return Allocate(count); } + static void operator delete(void* ptr) { free(ptr); } + static void operator delete[](void* ptr) { free(ptr); } + + private: + static void* Allocate(std::size_t count) { + void* result = nullptr; + const auto alloc_failed = posix_memalign(&result, ALIGNMENT, count); + if (alloc_failed) { + LOG(ERROR) << "Failed to allocate aligned memory; return code " << alloc_failed; + throw std::bad_alloc(); + } + return result; + } +}; + +using CacheLineAligned = AlignedNew<64>; +} + +#endif http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/benchmark-test.cc ---------------------------------------------------------------------- diff --git a/be/src/util/benchmark-test.cc b/be/src/util/benchmark-test.cc index 2c33e3b..fd1ed81 100644 --- a/be/src/util/benchmark-test.cc +++ b/be/src/util/benchmark-test.cc @@ -40,7 +40,7 @@ class BenchmarkTest { public: static double Measure(Benchmark::BenchmarkFunction fn, void* data) { return Benchmark::Measure(fn, data); - }; + } }; void TestFunction(int batch_size, void* d) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/bit-stream-utils.inline.h ---------------------------------------------------------------------- diff --git a/be/src/util/bit-stream-utils.inline.h b/be/src/util/bit-stream-utils.inline.h index 41648e3..dee2456 100644 --- a/be/src/util/bit-stream-utils.inline.h +++ b/be/src/util/bit-stream-utils.inline.h @@ -93,6 +93,8 @@ inline bool BitReader::GetValue(int num_bits, T* v) { if (UNLIKELY(byte_offset_ * 8 + bit_offset_ + num_bits > max_bytes_ * 8)) return false; + DCHECK_GE(bit_offset_, 0); + DCHECK_LE(bit_offset_, 64); *v = BitUtil::TrailingBits(buffered_values_, bit_offset_ + num_bits) >> bit_offset_; bit_offset_ += num_bits; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/bit-util-test.cc ---------------------------------------------------------------------- diff --git a/be/src/util/bit-util-test.cc b/be/src/util/bit-util-test.cc index 0c0db8b..663f10f 100644 --- a/be/src/util/bit-util-test.cc +++ b/be/src/util/bit-util-test.cc @@ -90,9 +90,9 @@ TEST(BitUtil, TrailingBits) { BOOST_BINARY(1 1 1 1 1 1 1 1)); EXPECT_EQ(BitUtil::TrailingBits(0, 1), 0); EXPECT_EQ(BitUtil::TrailingBits(0, 64), 0); - EXPECT_EQ(BitUtil::TrailingBits(1LL << 63, 0), 0); - EXPECT_EQ(BitUtil::TrailingBits(1LL << 63, 63), 0); - EXPECT_EQ(BitUtil::TrailingBits(1LL << 63, 64), 1LL << 63); + EXPECT_EQ(BitUtil::TrailingBits(1ULL << 63, 0), 0); + EXPECT_EQ(BitUtil::TrailingBits(1ULL << 63, 63), 0); + EXPECT_EQ(BitUtil::TrailingBits(1ULL << 63, 64), 1ULL << 63); } // Test different SIMD functionality units with an input/output buffer. @@ -141,6 +141,7 @@ void TestByteSwapSimd_Unit(const int64_t CpuFlag) { // CpuFlag == 0 for BitUtil::ByteSwap; // buf_size parameter indicates the size of input/output buffer. void TestByteSwapSimd(const int64_t CpuFlag, const int buf_size) { + if (buf_size <= 0) return; uint8_t src_buf[buf_size]; uint8_t dst_buf[buf_size]; std::iota(src_buf, src_buf + buf_size, 0); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/blocking-queue-test.cc ---------------------------------------------------------------------- diff --git a/be/src/util/blocking-queue-test.cc b/be/src/util/blocking-queue-test.cc index f45853a..f61de21 100644 --- a/be/src/util/blocking-queue-test.cc +++ b/be/src/util/blocking-queue-test.cc @@ -67,7 +67,7 @@ TEST(BlockingQueueTest, TestPutWithTimeout) { ASSERT_TRUE(test_queue.BlockingPutWithTimeout(3, timeout_micros)); } -class MultiThreadTest { +class MultiThreadTest { // NOLINT: members are not arranged for minimal padding public: MultiThreadTest() : iterations_(10000), http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/blocking-queue.h ---------------------------------------------------------------------- diff --git a/be/src/util/blocking-queue.h b/be/src/util/blocking-queue.h index e12793e..a762d5d 100644 --- a/be/src/util/blocking-queue.h +++ b/be/src/util/blocking-queue.h @@ -27,6 +27,7 @@ #include "common/atomic.h" #include "common/compiler-util.h" +#include "util/aligned-new.h" #include "util/condition-variable.h" #include "util/stopwatch.h" #include "util/time.h" @@ -43,7 +44,7 @@ namespace impala { /// will atomically swap the 'put_list_' with 'get_list_'. The swapping happens with both /// the 'get_lock_' and 'put_lock_' held. template <typename T> -class BlockingQueue { +class BlockingQueue : public CacheLineAligned { public: BlockingQueue(size_t max_elements) : shutdown_(false), @@ -233,7 +234,7 @@ class BlockingQueue { /// variable doesn't include the time which other threads block waiting for 'get_lock_'. int64_t total_get_wait_time_; -} CACHE_ALIGNED; +}; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/bloom-filter-test.cc ---------------------------------------------------------------------- diff --git a/be/src/util/bloom-filter-test.cc b/be/src/util/bloom-filter-test.cc index 9e63227..20c2655 100644 --- a/be/src/util/bloom-filter-test.cc +++ b/be/src/util/bloom-filter-test.cc @@ -180,7 +180,8 @@ TEST(BloomFilter, FindInvalid) { // Test that MaxNdv() and MinLogSpace() are dual TEST(BloomFilter, MinSpaceMaxNdv) { - for (double fpp = 0.25; fpp >= 1.0 / (1ull << 63); fpp /= 2) { + for (int log2fpp = -2; log2fpp >= -63; --log2fpp) { + const double fpp = pow(2, log2fpp); for (int given_log_space = 8; given_log_space < 30; ++given_log_space) { const size_t derived_ndv = BloomFilter::MaxNdv(given_log_space, fpp); int derived_log_space = BloomFilter::MinLogSpace(derived_ndv, fpp); @@ -225,7 +226,7 @@ TEST(BloomFilter, MinSpaceEdgeCase) { // Check that MinLogSpace() and FalsePositiveProb() are dual TEST(BloomFilter, MinSpaceForFpp) { for (size_t ndv = 10000; ndv < 100 * 1000 * 1000; ndv *= 1.01) { - for (double fpp = 0.1; fpp > pow(2, -20); fpp *= 0.99) { + for (double fpp = 0.1; fpp > pow(2, -20); fpp *= 0.99) { // NOLINT: loop on double // When contructing a Bloom filter, we can request a particular fpp by calling // MinLogSpace(). const int min_log_space = BloomFilter::MinLogSpace(ndv, fpp); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/buffer-builder.h ---------------------------------------------------------------------- diff --git a/be/src/util/buffer-builder.h b/be/src/util/buffer-builder.h index afcf9ae..a421e20 100644 --- a/be/src/util/buffer-builder.h +++ b/be/src/util/buffer-builder.h @@ -27,35 +27,31 @@ namespace impala { /// Utility class to build an in-memory buffer. class BufferBuilder { public: - BufferBuilder(uint8_t* dst_buffer, int dst_len) - : buffer_(dst_buffer), capacity_(dst_len), size_(0) { - } - - BufferBuilder(char* dst_buffer, int dst_len) - : buffer_(reinterpret_cast<uint8_t*>(dst_buffer)), - capacity_(dst_len), size_(0) { - } + BufferBuilder(uint8_t* dst_buffer, int dst_len) + : buffer_(dst_buffer), capacity_(dst_len), size_(0) {} + + BufferBuilder(char* dst_buffer, int dst_len) + : buffer_(reinterpret_cast<uint8_t*>(dst_buffer)), capacity_(dst_len), size_(0) {} - inline void Append(const void* buffer, int len) { + inline void Append(const void* buffer, int len) __attribute__((nonnull)) { DCHECK_LE(size_ + len, capacity_); memcpy(buffer_ + size_, buffer, len); size_ += len; } - template<typename T> + template <typename T> inline void Append(const T& v) { Append(&v, sizeof(T)); } int capacity() const { return capacity_; } int size() const { return size_; } - - private: + + private: uint8_t* buffer_; int capacity_; int size_; }; - } #endif http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/compress.cc ---------------------------------------------------------------------- diff --git a/be/src/util/compress.cc b/be/src/util/compress.cc index 7c97737..3737828 100644 --- a/be/src/util/compress.cc +++ b/be/src/util/compress.cc @@ -158,7 +158,7 @@ Status BzipCompressor::ProcessBlock(bool output_preallocated, int64_t input_leng out_buffer_ = temp_memory_pool_->Allocate(buffer_length_); } - unsigned int outlen; + unsigned int outlen = static_cast<unsigned int>(buffer_length_); int ret = BZ_OUTBUFF_FULL; while (ret == BZ_OUTBUFF_FULL) { if (out_buffer_ == NULL) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/decompress.cc ---------------------------------------------------------------------- diff --git a/be/src/util/decompress.cc b/be/src/util/decompress.cc index edb5490..7896c33 100644 --- a/be/src/util/decompress.cc +++ b/be/src/util/decompress.cc @@ -279,7 +279,7 @@ Status BzipDecompressor::ProcessBlock(bool output_preallocated, int64_t input_le } int ret = BZ_OUTBUFF_FULL; - unsigned int outlen; + unsigned int outlen = static_cast<unsigned int>(buffer_length_); // TODO: IMPALA-3073 Verify if compressed block could be multistream. If yes, we need // to support it and shouldn't stop decompressing while ret == BZ_STREAM_END. while (ret == BZ_OUTBUFF_FULL) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/metrics.h ---------------------------------------------------------------------- diff --git a/be/src/util/metrics.h b/be/src/util/metrics.h index 8b3942d..93497ea 100644 --- a/be/src/util/metrics.h +++ b/be/src/util/metrics.h @@ -60,7 +60,7 @@ class MetricDefs { /// Contains the map of all TMetricDefs, non-const for testing MetricDefsConstants metric_defs_; - MetricDefs() { }; + MetricDefs() { } DISALLOW_COPY_AND_ASSIGN(MetricDefs); }; @@ -190,8 +190,8 @@ class SimpleMetric : public Metric { document->AddMember(key_.c_str(), val, document->GetAllocator()); } - const TUnit::type unit() const { return unit_; } - const TMetricKind::type kind() const { return metric_kind; } + TUnit::type unit() const { return unit_; } + TMetricKind::type kind() const { return metric_kind; } protected: /// Called to compute value_ if necessary during calls to value(). The more natural http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/minidump.cc ---------------------------------------------------------------------- diff --git a/be/src/util/minidump.cc b/be/src/util/minidump.cc index 4567c48..b87ea90 100644 --- a/be/src/util/minidump.cc +++ b/be/src/util/minidump.cc @@ -29,6 +29,7 @@ #include <iomanip> #include <map> #include <signal.h> +#include <sstream> #include "common/logging.h" #include "common/version.h" @@ -41,15 +42,12 @@ using boost::filesystem::create_directories; using boost::filesystem::is_regular_file; using boost::filesystem::path; using boost::filesystem::remove; -using boost::system::error_code; DECLARE_string(log_dir); DECLARE_string(minidump_path); DECLARE_int32(max_minidumps); DECLARE_int32(minidump_size_limit_hint_kb); -#define MINIDUMP_LOG_BUF_SIZE 256 - namespace impala { /// Breakpad ExceptionHandler. It registers its own signal handlers to write minidump @@ -127,7 +125,7 @@ static void CheckAndRemoveMinidumps(int max_minidumps) { glob(pattern.c_str(), GLOB_TILDE, NULL, &result); for (size_t i = 0; i < result.gl_pathc; ++i) { const path minidump_path(result.gl_pathv[i]); - error_code err; + boost::system::error_code err; bool is_file = is_regular_file(minidump_path, err); // is_regular_file() calls stat() eventually, which can return errors, e.g. if the // file permissions prevented access or the path was wrong (see 'man 2 stat' for @@ -178,7 +176,7 @@ static void CheckAndRemoveMinidumps(int max_minidumps) { DCHECK_GT(files_to_delete, 0); auto to_delete = timestamp_to_path.begin(); for (int i = 0; i < files_to_delete; ++i, ++to_delete) { - error_code err; + boost::system::error_code err; remove(to_delete->second, err); if (!err) { LOG(INFO) << "Removed old minidump file : " << to_delete->second; @@ -209,7 +207,7 @@ Status RegisterMinidump(const char* cmd_line_path) { // Create the directory if it is not there. The minidump doesn't get written if there is // no directory. - error_code err; + boost::system::error_code err; create_directories(FLAGS_minidump_path, err); if (err) { stringstream ss; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/network-util.cc ---------------------------------------------------------------------- diff --git a/be/src/util/network-util.cc b/be/src/util/network-util.cc index afa5b32..0a722cc 100644 --- a/be/src/util/network-util.cc +++ b/be/src/util/network-util.cc @@ -23,6 +23,7 @@ #include <arpa/inet.h> #include <limits.h> #include <sstream> +#include <random> #include <vector> #include <boost/algorithm/string.hpp> http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/parquet-reader.cc ---------------------------------------------------------------------- diff --git a/be/src/util/parquet-reader.cc b/be/src/util/parquet-reader.cc index de11d7c..e6a9084 100644 --- a/be/src/util/parquet-reader.cc +++ b/be/src/util/parquet-reader.cc @@ -51,8 +51,7 @@ using namespace apache::thrift; using namespace parquet; using std::min; -// Some code is replicated to make this more stand-alone. -const uint8_t PARQUET_VERSION_NUMBER[] = {'P', 'A', 'R', '1'}; +using impala::PARQUET_VERSION_NUMBER; boost::shared_ptr<TProtocol> CreateDeserializeProtocol( boost::shared_ptr<TMemoryBuffer> mem, bool compact) { @@ -221,6 +220,7 @@ int main(int argc, char** argv) { uint8_t* buffer = reinterpret_cast<uint8_t*>(malloc(file_len)); size_t bytes_read = fread(buffer, 1, file_len, file); assert(bytes_read == file_len); + (void)bytes_read; // Check file starts and ends with magic bytes assert( http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/periodic-counter-updater.h ---------------------------------------------------------------------- diff --git a/be/src/util/periodic-counter-updater.h b/be/src/util/periodic-counter-updater.h index 161569c..893b887 100644 --- a/be/src/util/periodic-counter-updater.h +++ b/be/src/util/periodic-counter-updater.h @@ -102,7 +102,7 @@ class PeriodicCounterUpdater { /// Loop for periodic counter update thread. This thread wakes up once in a while /// and updates all the added rate counters and sampling counters. - void UpdateLoop(); + [[noreturn]] void UpdateLoop(); /// Thread performing asynchronous updates. boost::scoped_ptr<boost::thread> update_thread_; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/pprof-path-handlers.cc ---------------------------------------------------------------------- diff --git a/be/src/util/pprof-path-handlers.cc b/be/src/util/pprof-path-handlers.cc index 1c6f400..b8f245c 100644 --- a/be/src/util/pprof-path-handlers.cc +++ b/be/src/util/pprof-path-handlers.cc @@ -94,7 +94,7 @@ void PprofCpuProfileHandler(const Webserver::ArgumentMap& args, stringstream* ou ProfilerStop(); ifstream prof_file(tmp_prof_file_name.str().c_str(), ios::in); if (!prof_file.is_open()) { - (*output) << "Unable to open cpu profile: " << tmp_prof_file_name; + (*output) << "Unable to open cpu profile: " << tmp_prof_file_name.str(); return; } (*output) << prof_file.rdbuf(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/progress-updater.cc ---------------------------------------------------------------------- diff --git a/be/src/util/progress-updater.cc b/be/src/util/progress-updater.cc index ba0dfca..ad37510 100644 --- a/be/src/util/progress-updater.cc +++ b/be/src/util/progress-updater.cc @@ -52,7 +52,7 @@ void ProgressUpdater::Update(int64_t delta) { if (num_complete >= total_) { // Always print the final 100% complete - VLOG(logging_level_) << label_ << " 100\% Complete (" + VLOG(logging_level_) << label_ << " 100% Complete (" << num_complete << " out of " << total_ << ")"; return; } @@ -62,7 +62,7 @@ void ProgressUpdater::Update(int64_t delta) { if (new_percentage - old_percentage > update_period_) { // Only update shared variable if this guy was the latest. last_output_percentage_.CompareAndSwap(old_percentage, new_percentage); - VLOG(logging_level_) << label_ << ": " << new_percentage << "\% Complete (" + VLOG(logging_level_) << label_ << ": " << new_percentage << "% Complete (" << num_complete << " out of " << total_ << ")"; } } @@ -72,11 +72,11 @@ string ProgressUpdater::ToString() const { int64_t num_complete = num_complete_.Load(); if (num_complete >= total_) { // Always print the final 100% complete - ss << label_ << " 100\% Complete (" << num_complete << " out of " << total_ << ")"; + ss << label_ << " 100% Complete (" << num_complete << " out of " << total_ << ")"; return ss.str(); } int percentage = (static_cast<double>(num_complete) / total_) * 100; - ss << label_ << ": " << percentage << "\% Complete (" + ss << label_ << ": " << percentage << "% Complete (" << num_complete << " out of " << total_ << ")"; return ss.str(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/rle-test.cc ---------------------------------------------------------------------- diff --git a/be/src/util/rle-test.cc b/be/src/util/rle-test.cc index 0e0368c..0501d71 100644 --- a/be/src/util/rle-test.cc +++ b/be/src/util/rle-test.cc @@ -102,9 +102,9 @@ TEST(BitArray, TestBool) { // Writes 'num_vals' values with width 'bit_width' and reads them back. void TestBitArrayValues(int bit_width, int num_vals) { const int len = BitUtil::Ceil(bit_width * num_vals, 8); - const uint64_t mod = bit_width == 64? 1 : 1LL << bit_width; + const uint64_t mod = bit_width == 64 ? 1 : 1LL << bit_width; - uint8_t buffer[len]; + uint8_t buffer[(len > 0) ? len : 1]; BitWriter writer(buffer, len); for (int i = 0; i < num_vals; ++i) { bool result = writer.PutValue(i % mod, bit_width); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/runtime-profile.h ---------------------------------------------------------------------- diff --git a/be/src/util/runtime-profile.h b/be/src/util/runtime-profile.h index c409d62..a514f8b 100644 --- a/be/src/util/runtime-profile.h +++ b/be/src/util/runtime-profile.h @@ -41,7 +41,8 @@ class ObjectPool; /// corresponding rate based counter. This thread wakes up at fixed intervals and updates /// all of the rate counters. /// Thread-safe. -class RuntimeProfile { +class RuntimeProfile { // NOLINT: This struct is not packed, but there are not so many + // of them that it makes a performance difference public: class Counter { public: http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/thread-pool.h ---------------------------------------------------------------------- diff --git a/be/src/util/thread-pool.h b/be/src/util/thread-pool.h index d7be6dd..c5da7bd 100644 --- a/be/src/util/thread-pool.h +++ b/be/src/util/thread-pool.h @@ -23,6 +23,7 @@ #include <boost/thread/mutex.hpp> #include <boost/bind/mem_fn.hpp> +#include "util/aligned-new.h" #include "util/thread.h" namespace impala { @@ -30,7 +31,7 @@ namespace impala { /// Simple threadpool which processes items (of type T) in parallel which were placed on a /// blocking queue by Offer(). Each item is processed by a single user-supplied method. template <typename T> -class ThreadPool { +class ThreadPool : public CacheLineAligned { public: /// Signature of a work-processing function. Takes the integer id of the thread which is /// calling it (ids run from 0 to num_threads - 1) and a reference to the item to http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/be/src/util/webserver.cc ---------------------------------------------------------------------- diff --git a/be/src/util/webserver.cc b/be/src/util/webserver.cc index 41974bb..7cdd026 100644 --- a/be/src/util/webserver.cc +++ b/be/src/util/webserver.cc @@ -420,7 +420,13 @@ int Webserver::BeginRequestCallback(struct sq_connection* connection, const string& str = output.str(); const string& headers = BuildHeaderString(response, content_type); + + // printf with a non-literal format string is a security concern, but BuildHeaderString + // returns a limited set of strings and all members of that set are safe. +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wformat-nonliteral" sq_printf(connection, headers.c_str(), (int)str.length()); +#pragma clang diagnostic pop // Make sure to use sq_write for printing the body; sq_printf truncates at 8kb sq_write(connection, str.c_str(), str.length()); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/bin/make_impala.sh ---------------------------------------------------------------------- diff --git a/bin/make_impala.sh b/bin/make_impala.sh index eb3ecf5..c198378 100755 --- a/bin/make_impala.sh +++ b/bin/make_impala.sh @@ -115,8 +115,10 @@ then if [[ ! -z $IMPALA_TOOLCHAIN ]]; then - if [[ "$TARGET_BUILD_TYPE" == "ADDRESS_SANITIZER" ]]; then - CMAKE_ARGS+=(-DCMAKE_TOOLCHAIN_FILE=$IMPALA_HOME/cmake_modules/asan_toolchain.cmake) + if [[ ("$TARGET_BUILD_TYPE" == "ADDRESS_SANITIZER") \ + || ("$TARGET_BUILD_TYPE" == "TIDY") ]] + then + CMAKE_ARGS+=(-DCMAKE_TOOLCHAIN_FILE=$IMPALA_HOME/cmake_modules/clang_toolchain.cmake) else CMAKE_ARGS+=(-DCMAKE_TOOLCHAIN_FILE=$IMPALA_HOME/cmake_modules/toolchain.cmake) fi http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/bin/run_clang_tidy.sh ---------------------------------------------------------------------- diff --git a/bin/run_clang_tidy.sh b/bin/run_clang_tidy.sh new file mode 100755 index 0000000..aa95845 --- /dev/null +++ b/bin/run_clang_tidy.sh @@ -0,0 +1,54 @@ +#!/bin/bash + +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# This script runs clang-tidy on the backend, excluding gutil. clang-tidy finds bugs and +# likely bugs. +# +# To use this script, the toolchain must be installed and the toolchain environment +# variables must be set. +# +# The output of this script is quite verbose becuase clang-tidy and the clang-provided +# run-clang-tidy.py have verbose output. The lines indicating likely bugs are the ones +# that end in ']'. + +set -euo pipefail + +echo "Compiling" +if ! ./buildall.sh -notests -tidy -so -noclean 1>/dev/null 2>/dev/null +then + echo "WARNING: compile failed" >&2 +fi + +DIRS=$(ls -d "${IMPALA_HOME}/be/src/"*/ | grep -v gutil | tr '\n' ' ') +PIPE_DIRS=$(echo "${DIRS}" | tr ' ' '|') + +# Reduce the concurrency to one less than the number of cores in the system. Note than +# nproc may not be available on older distributions like Centos 5.5. +if type nproc >/dev/null 2>&1; then + CORES=$(($(nproc) - 1)) +else + # This script has been tested when CORES is actually higher than the number of cores on + # the system, and the system remained responsive, so it's OK to guess here. + CORES=7 +fi + +export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/share/clang\ +:${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin/\ +:$PATH" +run-clang-tidy.py -header-filter "${PIPE_DIRS%?}" -j"${CORES}" ${DIRS} http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/buildall.sh ---------------------------------------------------------------------- diff --git a/buildall.sh b/buildall.sh index d830ae3..a4791b7 100755 --- a/buildall.sh +++ b/buildall.sh @@ -58,6 +58,7 @@ MAKE_IMPALA_ARGS="" CODE_COVERAGE=0 BUILD_ASAN=0 BUILD_FE_ONLY=0 +BUILD_TIDY=0 MAKE_CMD=make LZO_CMAKE_ARGS= @@ -109,6 +110,9 @@ do -asan) BUILD_ASAN=1 ;; + -tidy) + BUILD_TIDY=1 + ;; -testpairwise) EXPLORATION_STRATEGY=pairwise ;; @@ -174,6 +178,7 @@ do echo "[-release] : Release build [Default: debug]" echo "[-codecoverage] : Build with code coverage [Default: False]" echo "[-asan] : Address sanitizer build [Default: False]" + echo "[-tidy] : clang-tidy build [Default: False]" echo "[-skiptests] : Skips execution of all tests" echo "[-notests] : Skips building and execution of all tests" echo "[-start_minicluster] : Start test cluster including Impala and all"\ @@ -248,6 +253,10 @@ if [[ ${BUILD_ASAN} -eq 1 ]]; then fi CMAKE_BUILD_TYPE=ADDRESS_SANITIZER fi +if [[ ${BUILD_TIDY} -eq 1 ]]; then + CMAKE_BUILD_TYPE=TIDY +fi + MAKE_IMPALA_ARGS+=" -build_type=${CMAKE_BUILD_TYPE}" # If we aren't kerberized then we certainly don't need to talk about http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/cmake_modules/asan_toolchain.cmake ---------------------------------------------------------------------- diff --git a/cmake_modules/asan_toolchain.cmake b/cmake_modules/asan_toolchain.cmake deleted file mode 100644 index 1500e87..0000000 --- a/cmake_modules/asan_toolchain.cmake +++ /dev/null @@ -1,44 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. - -# Set the root directory for the toolchain -set(TOOLCHAIN_ROOT $ENV{IMPALA_TOOLCHAIN}) - -# If Impala is built with the toolchain, change compiler and link paths -set(GCC_ROOT $ENV{IMPALA_TOOLCHAIN}/gcc-$ENV{IMPALA_GCC_VERSION}) - -# Use the appropriate LLVM version to build ASAN. -set(LLVM_ASAN_ROOT $ENV{IMPALA_TOOLCHAIN}/llvm-$ENV{IMPALA_LLVM_ASAN_VERSION}) - -set(CMAKE_C_COMPILER ${LLVM_ASAN_ROOT}/bin/clang) - -# Use clang to build unless overridden by environment. -if($ENV{IMPALA_CXX_COMPILER} STREQUAL "default") - set(CMAKE_CXX_COMPILER ${LLVM_ASAN_ROOT}/bin/clang++) -else() - set(CMAKE_CXX_COMPILER $ENV{IMPALA_CXX_COMPILER}) -endif() - -# Add the GCC root location to the compiler flags -set(CXX_COMMON_FLAGS "--gcc-toolchain=${GCC_ROOT}") - -# The rpath is needed to be able to run the binaries produced by the toolchain without -# specifying an LD_LIBRARY_PATH -set(TOOLCHAIN_LINK_FLAGS "-Wl,-rpath,${GCC_ROOT}/lib64") -set(TOOLCHAIN_LINK_FLAGS "${TOOLCHAIN_LINK_FLAGS} -L${GCC_ROOT}/lib64") - -message(STATUS "Setup toolchain link flags ${TOOLCHAIN_LINK_FLAGS}") http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/cmake_modules/clang_toolchain.cmake ---------------------------------------------------------------------- diff --git a/cmake_modules/clang_toolchain.cmake b/cmake_modules/clang_toolchain.cmake new file mode 100644 index 0000000..9a44c1d --- /dev/null +++ b/cmake_modules/clang_toolchain.cmake @@ -0,0 +1,54 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Set the root directory for the toolchain +set(TOOLCHAIN_ROOT $ENV{IMPALA_TOOLCHAIN}) + +# If Impala is built with the toolchain, change compiler and link paths +set(GCC_ROOT $ENV{IMPALA_TOOLCHAIN}/gcc-$ENV{IMPALA_GCC_VERSION}) + +# Use the appropriate LLVM version to build ASAN. +set(LLVM_ASAN_ROOT $ENV{IMPALA_TOOLCHAIN}/llvm-$ENV{IMPALA_LLVM_ASAN_VERSION}) + +set(LLVM_ROOT $ENV{IMPALA_TOOLCHAIN}/llvm-$ENV{IMPALA_LLVM_VERSION}) + +if ("${CMAKE_BUILD_TYPE}" STREQUAL "ADDRESS_SANITIZER") + set(CMAKE_C_COMPILER ${LLVM_ASAN_ROOT}/bin/clang) +else() + set(CMAKE_C_COMPILER ${LLVM_ROOT}/bin/clang) +endif() + +# Use clang to build unless overridden by environment. +if($ENV{IMPALA_CXX_COMPILER} STREQUAL "default") + if ("${CMAKE_BUILD_TYPE}" STREQUAL "ADDRESS_SANITIZER") + set(CMAKE_CXX_COMPILER ${LLVM_ASAN_ROOT}/bin/clang++) + else() + set(CMAKE_CXX_COMPILER ${LLVM_ROOT}/bin/clang++) + endif() +else() + set(CMAKE_CXX_COMPILER $ENV{IMPALA_CXX_COMPILER}) +endif() + +# Add the GCC root location to the compiler flags +set(CXX_COMMON_FLAGS "--gcc-toolchain=${GCC_ROOT}") + +# The rpath is needed to be able to run the binaries produced by the toolchain without +# specifying an LD_LIBRARY_PATH +set(TOOLCHAIN_LINK_FLAGS "-Wl,-rpath,${GCC_ROOT}/lib64") +set(TOOLCHAIN_LINK_FLAGS "${TOOLCHAIN_LINK_FLAGS} -L${GCC_ROOT}/lib64") + +message(STATUS "Setup toolchain link flags ${TOOLCHAIN_LINK_FLAGS}") http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/14891fe0/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java b/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java index 4fcecb2..93f859d 100644 --- a/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java +++ b/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java @@ -95,25 +95,25 @@ public class BuiltinsDb extends Db { private static final Map<Type, String> SAMPLE_SERIALIZE_SYMBOL = ImmutableMap.<Type, String>builder() .put(Type.BOOLEAN, - "24ReservoirSampleSerializeIN10impala_udf10BooleanValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_") + "24ReservoirSampleSerializeIN10impala_udf10BooleanValEEENS2_9StringValEPNS2_15FunctionContextERKS4_") .put(Type.TINYINT, - "24ReservoirSampleSerializeIN10impala_udf10TinyIntValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_") + "24ReservoirSampleSerializeIN10impala_udf10TinyIntValEEENS2_9StringValEPNS2_15FunctionContextERKS4_") .put(Type.SMALLINT, - "24ReservoirSampleSerializeIN10impala_udf11SmallIntValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_") + "24ReservoirSampleSerializeIN10impala_udf11SmallIntValEEENS2_9StringValEPNS2_15FunctionContextERKS4_") .put(Type.INT, - "24ReservoirSampleSerializeIN10impala_udf6IntValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_") + "24ReservoirSampleSerializeIN10impala_udf6IntValEEENS2_9StringValEPNS2_15FunctionContextERKS4_") .put(Type.BIGINT, - "24ReservoirSampleSerializeIN10impala_udf9BigIntValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_") + "24ReservoirSampleSerializeIN10impala_udf9BigIntValEEENS2_9StringValEPNS2_15FunctionContextERKS4_") .put(Type.FLOAT, - "24ReservoirSampleSerializeIN10impala_udf8FloatValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_") + "24ReservoirSampleSerializeIN10impala_udf8FloatValEEENS2_9StringValEPNS2_15FunctionContextERKS4_") .put(Type.DOUBLE, - "24ReservoirSampleSerializeIN10impala_udf9DoubleValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_") + "24ReservoirSampleSerializeIN10impala_udf9DoubleValEEENS2_9StringValEPNS2_15FunctionContextERKS4_") .put(Type.STRING, - "24ReservoirSampleSerializeIN10impala_udf9StringValEEEKS3_PNS2_15FunctionContextERS4_") + "24ReservoirSampleSerializeIN10impala_udf9StringValEEES3_PNS2_15FunctionContextERKS3_") .put(Type.TIMESTAMP, - "24ReservoirSampleSerializeIN10impala_udf12TimestampValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_") + "24ReservoirSampleSerializeIN10impala_udf12TimestampValEEENS2_9StringValEPNS2_15FunctionContextERKS4_") .put(Type.DECIMAL, - "24ReservoirSampleSerializeIN10impala_udf10DecimalValEEEKNS2_9StringValEPNS2_15FunctionContextERS5_") + "24ReservoirSampleSerializeIN10impala_udf10DecimalValEEENS2_9StringValEPNS2_15FunctionContextERKS4_") .build(); private static final Map<Type, String> SAMPLE_MERGE_SYMBOL =
