You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by wz...@apache.org on 2022/02/16 17:18:35 UTC
[impala] 03/03: IMPALA-10931 (part2): Fixed rebased Kudu source to compile
This is an automated email from the ASF dual-hosted git repository.
wzhou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 92ce6fe48e75d7780efe9a275122554e59aac916
Author: wzhou-code <wz...@cloudera.com>
AuthorDate: Wed Jan 12 21:20:36 2022 -0800
IMPALA-10931 (part2): Fixed rebased Kudu source to compile
This patch applies various fixes to Impala and to the copied Kudu
source code in be/src/kudu/* to allow everything to compile and pass
Impala exhaustive tests.
Highlights of the changes made:
- Some flags that have a DEFINE in both Kudu and Impala are modified
to change one of the DEFINEs to a DECLARE.
- Fixed backend unit-test bloom-filter-test.cc due to the changes for
block bloom filter false positive rate correction in
kudu/util/block_bloom_filter.cc.
- Set initial values of Sockaddr variables as Sockaddr::Wildcard().
- The parameter type of kudu::RpcSidecar::FromFaststring() was changed,
need to change the caller code in Impala.
- openssl_util.h was moved from directory kudu/security to kudu/util.
need to change including path in Impala.
This patch was in part based on the patches that were applied the last
time we rebased the Kudu code in IMPALA-9335. All changes from those
commits that are still relevant were included here.
Went through all commits that have been applied to the be/src/kudu
directory since the last rebase and ensured that all relevant changes
from those are included here. These include patches for IMPALA-9182,
IMPALA-10779, IMPALA-9940 and commit d9a38c0bac (Limit short versions
of KUDU_* macros to Kudu source).
Testing:
- Passed core debug build, core ASAN build and exhaustive release
build.
Change-Id: Ia9919c06e60d132d997093abb7b14825847e07c7
Reviewed-on: http://gerrit.cloudera.org:8080/18155
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/kudu/rpc/CMakeLists.txt | 26 ++++++++++----
be/src/kudu/rpc/transfer.cc | 4 ++-
be/src/kudu/security/CMakeLists.txt | 15 ++++++++
be/src/kudu/security/init.cc | 11 +++---
be/src/kudu/security/test/mini_kdc.cc | 8 +++--
be/src/kudu/security/tls_handshake.cc | 2 +-
be/src/kudu/util/CMakeLists.txt | 62 ++++++++++++++++++++++----------
be/src/kudu/util/flags.cc | 5 ++-
be/src/kudu/util/kudu_export.h | 62 ++++++++++++++++++++++++++++++++
be/src/kudu/util/logging.cc | 13 ++++---
be/src/kudu/util/logging.h | 2 +-
be/src/kudu/util/random_util.h | 5 ++-
be/src/kudu/util/web_callback_registry.h | 8 +++++
be/src/rpc/authentication-util.cc | 2 +-
be/src/rpc/authentication.cc | 2 +-
be/src/rpc/rpc-mgr.cc | 2 +-
be/src/rpc/rpc-mgr.inline.h | 2 +-
be/src/rpc/sidecar-util.h | 4 +--
be/src/rpc/thrift-server.cc | 2 +-
be/src/runtime/query-state.cc | 4 +--
be/src/util/bloom-filter-test.cc | 50 ++++++++++++++++----------
be/src/util/webserver.cc | 3 ++
22 files changed, 217 insertions(+), 77 deletions(-)
diff --git a/be/src/kudu/rpc/CMakeLists.txt b/be/src/kudu/rpc/CMakeLists.txt
index c8d831e..6195e33 100644
--- a/be/src/kudu/rpc/CMakeLists.txt
+++ b/be/src/kudu/rpc/CMakeLists.txt
@@ -15,6 +15,9 @@
# specific language governing permissions and limitations
# under the License.
+# Target including all protobuf-generated code.
+add_custom_target(kudu-rpc-proto-deps)
+
#### Global header protobufs
PROTOBUF_GENERATE_CPP(
RPC_HEADER_PROTO_SRCS RPC_HEADER_PROTO_HDRS RPC_HEADER_PROTO_TGTS
@@ -26,6 +29,8 @@ ADD_EXPORTABLE_LIBRARY(rpc_header_proto
DEPS protobuf pb_util_proto token_proto
NONLINK_DEPS ${RPC_HEADER_PROTO_TGTS})
+add_dependencies(kudu-rpc-proto-deps ${RPC_HEADER_PROTO_TGTS})
+
PROTOBUF_GENERATE_CPP(
RPC_INTROSPECTION_PROTO_SRCS RPC_INTROSPECTION_PROTO_HDRS RPC_INTROSPECTION_PROTO_TGTS
SOURCE_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/../..
@@ -39,6 +44,10 @@ ADD_EXPORTABLE_LIBRARY(rpc_introspection_proto
DEPS ${RPC_INTROSPECTION_PROTO_LIBS}
NONLINK_DEPS ${RPC_INTROSPECTION_PROTO_TGTS})
+add_dependencies(kudu-rpc-proto-deps ${RPC_INTROSPECTION_PROTO_TGTS})
+
+add_definitions(-DKUDU_HEADERS_USE_SHORT_STATUS_MACROS)
+
### RPC library
set(KRPC_SRCS
acceptor_pool.cc
@@ -90,13 +99,16 @@ ADD_EXPORTABLE_LIBRARY(krpc
DEPS ${KRPC_LIBS})
### RPC generator tool
-add_executable(protoc-gen-krpc protoc-gen-krpc.cc)
-target_link_libraries(protoc-gen-krpc
- ${KUDU_BASE_LIBS}
- rpc_header_proto
- protoc
- protobuf
- gutil)
+add_executable(protoc-gen-krpc protoc-gen-krpc.cc
+ # Impala - add stub for kudu::VersionInfo
+ ${CMAKE_CURRENT_SOURCE_DIR}/../../common/kudu_version.cc
+ # Impala - add definition for any flag names shared between Impala / Kudu.
+ # TODO: Consider either removing code that depends on these flags, or namespacing them
+ # somehow.
+ ${CMAKE_CURRENT_SOURCE_DIR}/../../common/global-flags.cc)
+# IMPALA-8642: kudu_version.cc depends on gen-cpp/Status_types.h in target thrift-deps
+add_dependencies(protoc-gen-krpc thrift-deps)
+target_link_libraries(protoc-gen-krpc gutil glog gflags protoc protobuf rpc_header_proto ${KUDU_BASE_LIBS})
#### RPC test
PROTOBUF_GENERATE_CPP(
diff --git a/be/src/kudu/rpc/transfer.cc b/be/src/kudu/rpc/transfer.cc
index cdb3b5e..04043aa 100644
--- a/be/src/kudu/rpc/transfer.cc
+++ b/be/src/kudu/rpc/transfer.cc
@@ -43,7 +43,9 @@ DEFINE_bool(rpc_max_message_size_enable_validation, true,
"This is a test-only flag.");
TAG_FLAG(rpc_max_message_size_enable_validation, unsafe);
-DEFINE_int64(rpc_max_message_size, (50 * 1024 * 1024),
+// Hidden in Impala because we require a particular value and override the user specified
+// value anyways, see RpcMgr::Init() and IMPALA-4874.
+DEFINE_int64_hidden(rpc_max_message_size, (50 * 1024 * 1024),
"The maximum size of a message that any RPC that the server will accept. "
"Must be at least 1MB.");
TAG_FLAG(rpc_max_message_size, advanced);
diff --git a/be/src/kudu/security/CMakeLists.txt b/be/src/kudu/security/CMakeLists.txt
index e4ddbaa..97bd81a 100644
--- a/be/src/kudu/security/CMakeLists.txt
+++ b/be/src/kudu/security/CMakeLists.txt
@@ -19,6 +19,9 @@
# The top-level CMakeLists sets a ${KRB5_REALM_OVERRIDE} variable which should
# be linked first into all Kudu binaries.
+# Target including all protobuf-generated code.
+add_custom_target(kudu-security-proto-deps)
+
##############################
# krb5_realm_override
##############################
@@ -29,6 +32,8 @@ if(NOT APPLE)
target_link_libraries(krb5_realm_override dl)
endif()
+add_definitions(-DKUDU_HEADERS_USE_SHORT_STATUS_MACROS)
+
##############################
# token_proto
##############################
@@ -44,6 +49,7 @@ ADD_EXPORTABLE_LIBRARY(token_proto
DEPS ${TOKEN_PROTO_LIBS}
NONLINK_DEPS ${TOKEN_PROTO_TGTS})
+add_dependencies(kudu-security-proto-deps ${TOKEN_PROTO_TGTS})
##############################
# security
@@ -95,6 +101,15 @@ ADD_EXPORTABLE_LIBRARY(security
SRCS ${SECURITY_SRCS}
DEPS ${SECURITY_LIBS})
+# Since Kudu tests are explicitly disabled, we want to expose some of their sources
+# to Impala using another variable.
+set(SECURITY_TEST_SRCS_FOR_IMPALA test/mini_kdc.cc)
+add_library(security-test-for-impala ${SECURITY_TEST_SRCS_FOR_IMPALA})
+target_link_libraries(security-test-for-impala
+ gutil
+ kudu_test_util
+ kudu_util
+ security)
##############################
# mini_kdc
diff --git a/be/src/kudu/security/init.cc b/be/src/kudu/security/init.cc
index 8d59d51..d99bebc 100644
--- a/be/src/kudu/security/init.cc
+++ b/be/src/kudu/security/init.cc
@@ -73,16 +73,13 @@ DEFINE_bool(use_system_auth_to_local, kDefaultSystemAuthToLocal,
"'kudu/foo.example.com@EXAMPLE' will map to 'kudu'.");
TAG_FLAG(use_system_auth_to_local, advanced);
-DEFINE_string(principal, "kudu/_HOST",
- "Kerberos principal that this daemon will log in as. The special token "
- "_HOST will be replaced with the FQDN of the local host.");
+// Defined in Impala in common/global-flags.cc
+DECLARE_string(principal);
TAG_FLAG(principal, advanced);
TAG_FLAG(principal, stable);
-DEFINE_string(keytab_file, "",
- "Path to the Kerberos Keytab file for this server. Specifying a "
- "keytab file will cause the server to kinit, and enable Kerberos "
- "to be used to authenticate RPC connections.");
+// Defined in Impala in common/global-flags.cc
+DECLARE_string(keytab_file);
TAG_FLAG(keytab_file, stable);
using std::mt19937;
diff --git a/be/src/kudu/security/test/mini_kdc.cc b/be/src/kudu/security/test/mini_kdc.cc
index 831699c..3cab6f5 100644
--- a/be/src/kudu/security/test/mini_kdc.cc
+++ b/be/src/kudu/security/test/mini_kdc.cc
@@ -65,7 +65,9 @@ MiniKdc::MiniKdc(MiniKdcOptions options)
options_.realm = "KRBTEST.COM";
}
if (options_.data_root.empty()) {
- options_.data_root = JoinPathSegments(GetTestDataDirectory(), "krb5kdc");
+ // We hardcode "/tmp" here since the original function which initializes a random test
+ // directory (GetTestDataDirectory()), depends on gmock.
+ options_.data_root = JoinPathSegments("/tmp", "krb5kdc");
}
if (options_.ticket_lifetime.empty()) {
options_.ticket_lifetime = "24h";
@@ -309,7 +311,9 @@ Status MiniKdc::Kinit(const string& username) {
RETURN_NOT_OK(GetBinaryPath("kinit", &kinit));
unique_ptr<WritableFile> tmp_cc_file;
string tmp_cc_path;
- const auto tmp_template = Substitute("kinit-temp-$0.XXXXXX", username);
+ string tmp_username = username;
+ StripString(&tmp_username, "/", '_');
+ const auto tmp_template = Substitute("kinit-temp-$0.XXXXXX", tmp_username);
WritableFileOptions opts;
opts.is_sensitive = false;
RETURN_NOT_OK_PREPEND(Env::Default()->NewTempWritableFile(
diff --git a/be/src/kudu/security/tls_handshake.cc b/be/src/kudu/security/tls_handshake.cc
index c313308..82e22e0 100644
--- a/be/src/kudu/security/tls_handshake.cc
+++ b/be/src/kudu/security/tls_handshake.cc
@@ -313,7 +313,7 @@ Status TlsHandshake::Finish(unique_ptr<Socket>* socket) {
if (data_size != 0) {
int fd = SSL_get_fd(ssl);
Socket sock(fd);
- uint8_t* data = reinterpret_cast<uint8_t*>(rbio_pending_data_.data());
+ const uint8_t* data = reinterpret_cast<const uint8_t*>(rbio_pending_data_.data());
int32_t written = 0;
RETURN_NOT_OK(sock.Write(data, data_size, &written));
if (written != data_size) {
diff --git a/be/src/kudu/util/CMakeLists.txt b/be/src/kudu/util/CMakeLists.txt
index ea13742..b273367 100644
--- a/be/src/kudu/util/CMakeLists.txt
+++ b/be/src/kudu/util/CMakeLists.txt
@@ -15,6 +15,9 @@
# specific language governing permissions and limitations
# under the License.
+# Target including all protobuf-generated code.
+add_custom_target(kudu-util-proto-deps)
+
#######################################
# block_bloom_filter_proto
#######################################
@@ -29,6 +32,8 @@ ADD_EXPORTABLE_LIBRARY(block_bloom_filter_proto
DEPS hash_proto pb_util_proto protobuf
NONLINK_DEPS ${BLOCK_BLOOM_FILTER_PROTO_TGTS})
+add_dependencies(kudu-util-proto-deps ${BLOCK_BLOOM_FILTER_PROTO_TGTS})
+
#######################################
# util_compression_proto
#######################################
@@ -43,6 +48,10 @@ ADD_EXPORTABLE_LIBRARY(util_compression_proto
DEPS protobuf
NONLINK_DEPS ${UTIL_COMPRESSION_PROTO_TGTS})
+add_dependencies(kudu-util-proto-deps ${UTIL_COMPRESSION_PROTO_TGTS})
+
+add_definitions(-DKUDU_HEADERS_USE_SHORT_STATUS_MACROS)
+
#######################################
# hash_proto
#######################################
@@ -57,6 +66,8 @@ ADD_EXPORTABLE_LIBRARY(hash_proto
DEPS protobuf
NONLINK_DEPS ${HASH_PROTO_TGTS})
+add_dependencies(kudu-util-proto-deps ${HASH_PROTO_TGTS})
+
#######################################
# histogram_proto
#######################################
@@ -71,6 +82,8 @@ ADD_EXPORTABLE_LIBRARY(histogram_proto
DEPS protobuf
NONLINK_DEPS ${HISTOGRAM_PROTO_TGTS})
+add_dependencies(kudu-util-proto-deps ${HISTOGRAM_PROTO_TGTS})
+
#######################################
# maintenance_manager_proto
#######################################
@@ -85,6 +98,8 @@ ADD_EXPORTABLE_LIBRARY(maintenance_manager_proto
DEPS protobuf
NONLINK_DEPS ${MAINTENANCE_MANAGER_PROTO_TGTS})
+add_dependencies(kudu-util-proto-deps ${MAINTENANCE_MANAGER_PROTO_TGTS})
+
#######################################
# mem_tracker_proto
#######################################
@@ -99,6 +114,8 @@ ADD_EXPORTABLE_LIBRARY(mem_tracker_proto
DEPS protobuf
NONLINK_DEPS ${MEM_TRACKER_PROTO_TGTS})
+add_dependencies(kudu-util-proto-deps ${MEM_TRACKER_PROTO_TGTS})
+
#######################################
# pb_util_proto
#######################################
@@ -113,6 +130,8 @@ ADD_EXPORTABLE_LIBRARY(pb_util_proto
DEPS protobuf
NONLINK_DEPS ${PB_UTIL_PROTO_TGTS})
+add_dependencies(kudu-util-proto-deps ${PB_UTIL_PROTO_TGTS})
+
#######################################
# version_info_proto
#######################################
@@ -127,6 +146,8 @@ ADD_EXPORTABLE_LIBRARY(version_info_proto
DEPS protobuf
NONLINK_DEPS ${VERSION_INFO_PROTO_TGTS})
+add_dependencies(kudu-util-proto-deps ${VERSION_INFO_PROTO_TGTS})
+
############################################################
# Version stamp
############################################################
@@ -165,7 +186,7 @@ set(UTIL_SRCS
block_bloom_filter.cc
bloom_filter.cc
cache.cc
- char_util.cc
+ #char_util.cc
coding.cc
condition_variable.cc
cow_object.cc
@@ -181,7 +202,7 @@ set(UTIL_SRCS
errno.cc
faststring.cc
fault_injection.cc
- file_cache.cc
+ #file_cache.cc
file_cache_metrics.cc
flags.cc
flag_tags.cc
@@ -205,7 +226,7 @@ set(UTIL_SRCS
memory/overwrite.cc
mem_tracker.cc
metrics.cc
- minidump.cc
+ #minidump.cc
monotime.cc
mutex.cc
net/dns_resolver.cc
@@ -245,11 +266,13 @@ set(UTIL_SRCS
trace_metrics.cc
user.cc
url-coding.cc
- version_info.cc
+ # Remove from compilation, as it depends on generated method calls. Replaced by
+ # kudu_version.cc in Impala's common library.
+ #version_info.cc
version_util.cc
web_callback_registry.cc
website_util.cc
- yamlreader.cc
+ #yamlreader.cc
zlib.cc
)
@@ -296,7 +319,7 @@ set(UTIL_LIBS
openssl_crypto
openssl_ssl
version_info_proto
- yaml
+ #yaml
zlib)
if(NOT APPLE)
@@ -339,7 +362,7 @@ ADD_EXPORTABLE_LIBRARY(kudu_util_compression
# Define LZ4_DISABLE_DEPRECATE_WARNINGS to mute warnings like:
# "'int LZ4_compress(const char*, char*, int)' is deprecated".
target_compile_definitions(kudu_util_compression PUBLIC LZ4_DISABLE_DEPRECATE_WARNINGS)
-target_compile_definitions(kudu_util_compression_exported PUBLIC LZ4_DISABLE_DEPRECATE_WARNINGS)
+#target_compile_definitions(kudu_util_compression_exported PUBLIC LZ4_DISABLE_DEPRECATE_WARNINGS)
#######################################
# kudu_curl_util
@@ -355,11 +378,11 @@ target_link_libraries(kudu_curl_util
#######################################
# kudu_cloud_util
#######################################
-add_library(kudu_cloud_util
- cloud/instance_detector.cc
- cloud/instance_metadata.cc)
-target_link_libraries(kudu_cloud_util
- kudu_curl_util)
+#add_library(kudu_cloud_util
+# cloud/instance_detector.cc
+# cloud/instance_metadata.cc)
+#target_link_libraries(kudu_cloud_util
+# kudu_curl_util)
# See the comment in sanitizer_options.cc for details on this library's usage.
# The top-level CMakeLists sets a ${SANITIZER_OPTIONS_OVERRIDE} variable which
@@ -394,7 +417,8 @@ add_library(kudu_test_util
target_link_libraries(kudu_test_util
gflags
glog
- gmock
+ # Impala doesn't have gmock in its toolchain
+ #gmock
gtest
kudu_util)
@@ -425,7 +449,7 @@ endif()
#######################################
add_executable(protoc-gen-insertions protoc-gen-insertions.cc)
-target_link_libraries(protoc-gen-insertions gutil protobuf protoc ${KUDU_BASE_LIBS})
+target_link_libraries(protoc-gen-insertions gutil glog gflags protoc protobuf ${KUDU_BASE_LIBS})
#######################################
# Unit tests
@@ -614,8 +638,8 @@ endif()
#######################################
# instance_detector-test
#######################################
-ADD_KUDU_TEST(cloud/instance_detector-test)
-if(NOT NO_TESTS)
- target_link_libraries(instance_detector-test
- kudu_cloud_util)
-endif()
+#ADD_KUDU_TEST(cloud/instance_detector-test)
+#if(NOT NO_TESTS)
+# target_link_libraries(instance_detector-test
+# kudu_cloud_util)
+#endif()
diff --git a/be/src/kudu/util/flags.cc b/be/src/kudu/util/flags.cc
index 8897689..ea6d842 100644
--- a/be/src/kudu/util/flags.cc
+++ b/be/src/kudu/util/flags.cc
@@ -81,9 +81,8 @@ DEFINE_bool(dump_metrics_xml, false,
TAG_FLAG(dump_metrics_xml, hidden);
#ifdef TCMALLOC_ENABLED
-DEFINE_bool(enable_process_lifetime_heap_profiling, false, "Enables heap "
- "profiling for the lifetime of the process. Profile output will be stored in the "
- "directory specified by -heap_profile_path.");
+// Defined in Impala in common/global-flags.cc
+DECLARE_bool(enable_process_lifetime_heap_profiling);
TAG_FLAG(enable_process_lifetime_heap_profiling, stable);
TAG_FLAG(enable_process_lifetime_heap_profiling, advanced);
diff --git a/be/src/kudu/util/kudu_export.h b/be/src/kudu/util/kudu_export.h
new file mode 100644
index 0000000..3cbdf11
--- /dev/null
+++ b/be/src/kudu/util/kudu_export.h
@@ -0,0 +1,62 @@
+// 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 file is generated during Kudu's build. Instead of recreating the necessary steps
+// in Impala's build process, we copy it into our repository. See
+// kudu/client/CMakeLists.txt in Kudu's repository for details.
+
+#ifndef KUDU_EXPORT_H
+#define KUDU_EXPORT_H
+
+#ifdef KUDU_STATIC_DEFINE
+# define KUDU_EXPORT
+# define KUDU_NO_EXPORT
+#else
+# ifndef KUDU_EXPORT
+# ifdef kudu_client_exported_EXPORTS
+ /* We are building this library */
+# define KUDU_EXPORT __attribute__((visibility("default")))
+# else
+ /* We are using this library */
+# define KUDU_EXPORT __attribute__((visibility("default")))
+# endif
+# endif
+
+# ifndef KUDU_NO_EXPORT
+# define KUDU_NO_EXPORT __attribute__((visibility("hidden")))
+# endif
+#endif
+
+#ifndef KUDU_DEPRECATED
+# define KUDU_DEPRECATED __attribute__ ((__deprecated__))
+#endif
+
+#ifndef KUDU_DEPRECATED_EXPORT
+# define KUDU_DEPRECATED_EXPORT KUDU_EXPORT KUDU_DEPRECATED
+#endif
+
+#ifndef KUDU_DEPRECATED_NO_EXPORT
+# define KUDU_DEPRECATED_NO_EXPORT KUDU_NO_EXPORT KUDU_DEPRECATED
+#endif
+
+#if 0 /* DEFINE_NO_DEPRECATED */
+# ifndef KUDU_NO_DEPRECATED
+# define KUDU_NO_DEPRECATED
+# endif
+#endif
+
+#endif
diff --git a/be/src/kudu/util/logging.cc b/be/src/kudu/util/logging.cc
index 3ec98f2..ff701cc 100644
--- a/be/src/kudu/util/logging.cc
+++ b/be/src/kudu/util/logging.cc
@@ -49,9 +49,8 @@
#include "kudu/util/signal.h"
#include "kudu/util/status.h"
-DEFINE_string(log_filename, "",
- "Prefix of log filename - "
- "full path is <log_dir>/<log_filename>.[INFO|WARN|ERROR|FATAL]");
+// Defined in Impala.
+DECLARE_string(log_filename);
TAG_FLAG(log_filename, stable);
DEFINE_bool(log_async, true,
@@ -64,9 +63,8 @@ DEFINE_int32(log_async_buffer_bytes_per_level, 2 * 1024 * 1024,
"level. Only relevant when --log_async is enabled.");
TAG_FLAG(log_async_buffer_bytes_per_level, hidden);
-DEFINE_int32(max_log_files, 10,
- "Maximum number of log files to retain per severity level. The most recent "
- "log files are retained. If set to 0, all log files are retained.");
+// Defined in Impala.
+DECLARE_int32(max_log_files);
TAG_FLAG(max_log_files, runtime);
TAG_FLAG(max_log_files, stable);
@@ -276,7 +274,8 @@ void InitGoogleLoggingSafe(const char* arg) {
IgnoreSigPipe();
// For minidump support. Must be called before logging threads started.
- CHECK_OK(BlockSigUSR1());
+ // Disabled by Impala, which does not link Kudu's minidump library.
+ //CHECK_OK(BlockSigUSR1());
if (FLAGS_log_async) {
EnableAsyncLogging();
diff --git a/be/src/kudu/util/logging.h b/be/src/kudu/util/logging.h
index f8b03b5..8e2241f 100644
--- a/be/src/kudu/util/logging.h
+++ b/be/src/kudu/util/logging.h
@@ -162,7 +162,7 @@ class ScopedDisableRedaction {
&google::LogMessage::SendToLog).stream()
#define KLOG_EVERY_N_SECS(severity, n_secs) \
- static logging::LogThrottler LOG_THROTTLER; \
+ static ::kudu::logging::LogThrottler LOG_THROTTLER; \
KLOG_EVERY_N_SECS_THROTTLER(severity, n_secs, LOG_THROTTLER, "no-tag")
#define WARN_NOT_OK_EVERY_N_SECS(to_call, warning_prefix, n_secs) do { \
diff --git a/be/src/kudu/util/random_util.h b/be/src/kudu/util/random_util.h
index 4448023..20b9c3f 100644
--- a/be/src/kudu/util/random_util.h
+++ b/be/src/kudu/util/random_util.h
@@ -27,7 +27,10 @@
#include <glog/logging.h>
#include "kudu/gutil/map-util.h"
-#include "kudu/util/int128_util.h"
+// Inline functions defined in int128_util.h cause ambiguous overload error for
+// std::ostream& operator<<(std::ostream& os, const unsigned __int128& val),
+// include "kudu/util/int128.h" instead.
+#include "kudu/util/int128.h"
#include "kudu/util/random.h"
namespace kudu {
diff --git a/be/src/kudu/util/web_callback_registry.h b/be/src/kudu/util/web_callback_registry.h
index 39a8062..2b7127d 100644
--- a/be/src/kudu/util/web_callback_registry.h
+++ b/be/src/kudu/util/web_callback_registry.h
@@ -64,6 +64,14 @@ class WebCallbackRegistry {
// In the case of a POST, the posted data.
std::string post_data;
+
+ // The socket address of the requester, <host>:<port>.
+ // Define this variable for IMPALA-9182.
+ std::string source_socket;
+
+ // Authenticated user, or 'anonymous' if no auth used
+ // Define this variable for IMPALA-10779.
+ std::string source_user = "anonymous";
};
// A response to an HTTP request whose body is rendered by template.
diff --git a/be/src/rpc/authentication-util.cc b/be/src/rpc/authentication-util.cc
index 64d0215..b02a97e 100644
--- a/be/src/rpc/authentication-util.cc
+++ b/be/src/rpc/authentication-util.cc
@@ -171,7 +171,7 @@ bool IsTrustedDomain(const std::string& origin, const std::string& trusted_domai
if (trusted_domain.empty()) return false;
vector<string> split = Split(origin, delimiter::Limit(",", 1));
if (split.empty()) return false;
- kudu::Sockaddr sock_addr;
+ kudu::Sockaddr sock_addr = kudu::Sockaddr::Wildcard();
kudu::Status s = sock_addr.ParseString(split[0], 0);
string host_name;
if (!s.ok()) {
diff --git a/be/src/rpc/authentication.cc b/be/src/rpc/authentication.cc
index f3bb08b..9eb6432 100644
--- a/be/src/rpc/authentication.cc
+++ b/be/src/rpc/authentication.cc
@@ -45,7 +45,7 @@
#include "kudu/rpc/sasl_common.h"
#include "kudu/security/gssapi.h"
#include "kudu/security/init.h"
-#include "kudu/security/openssl_util.h"
+#include "kudu/util/openssl_util.h"
#include "rpc/auth-provider.h"
#include "rpc/authentication-util.h"
#include "rpc/thrift-server.h"
diff --git a/be/src/rpc/rpc-mgr.cc b/be/src/rpc/rpc-mgr.cc
index b7a9813..761b7f9 100644
--- a/be/src/rpc/rpc-mgr.cc
+++ b/be/src/rpc/rpc-mgr.cc
@@ -201,7 +201,7 @@ Status RpcMgr::StartServices() {
DCHECK(!services_started_) << "May not call StartServices() twice";
// Convert 'address_' to Kudu's Sockaddr
- Sockaddr sockaddr;
+ Sockaddr sockaddr = Sockaddr::Wildcard();
if (FLAGS_rpc_use_loopback) {
// Listen on all addresses, including loopback.
sockaddr.set_port(address_.port);
diff --git a/be/src/rpc/rpc-mgr.inline.h b/be/src/rpc/rpc-mgr.inline.h
index e519c9f..8053f3b 100644
--- a/be/src/rpc/rpc-mgr.inline.h
+++ b/be/src/rpc/rpc-mgr.inline.h
@@ -46,7 +46,7 @@ Status RpcMgr::GetProxy(const TNetworkAddress& address, const std::string& hostn
address_to_use.hostname == ExecEnv::GetInstance()->krpc_address().hostname) {
address_to_use.__set_hostname(LOCALHOST_IP_STR);
}
- kudu::Sockaddr sockaddr;
+ kudu::Sockaddr sockaddr = kudu::Sockaddr::Wildcard();
RETURN_IF_ERROR(TNetworkAddressToSockaddr(address_to_use, &sockaddr));
proxy->reset(new P(messenger_, sockaddr, hostname));
diff --git a/be/src/rpc/sidecar-util.h b/be/src/rpc/sidecar-util.h
index 47b15f6..d1d7956 100644
--- a/be/src/rpc/sidecar-util.h
+++ b/be/src/rpc/sidecar-util.h
@@ -82,8 +82,8 @@ Status SetFaststringSidecar(const T& obj, RPC* rpc, int* sidecar_idx) {
if (serialized_len > FLAGS_rpc_max_message_size) {
return Status("Serialized sidecar exceeds --rpc_max_message_size.");
}
- std::unique_ptr<kudu::faststring> sidecar_str = std::make_unique<kudu::faststring>();
- sidecar_str->assign_copy(serialized_buf, serialized_len);
+ kudu::faststring sidecar_str;
+ sidecar_str.assign_copy(serialized_buf, serialized_len);
std::unique_ptr<kudu::rpc::RpcSidecar> rpc_sidecar =
kudu::rpc::RpcSidecar::FromFaststring(std::move(sidecar_str));
RETURN_IF_ERROR(FromKuduStatus(
diff --git a/be/src/rpc/thrift-server.cc b/be/src/rpc/thrift-server.cc
index 4204ded..8dd4dd7 100644
--- a/be/src/rpc/thrift-server.cc
+++ b/be/src/rpc/thrift-server.cc
@@ -32,7 +32,7 @@
#include <sstream>
#include "gen-cpp/Types_types.h"
-#include "kudu/security/openssl_util.h"
+#include "kudu/util/openssl_util.h"
#include "rpc/TAcceptQueueServer.h"
#include "rpc/auth-provider.h"
#include "rpc/authentication.h"
diff --git a/be/src/runtime/query-state.cc b/be/src/runtime/query-state.cc
index 1b36fe9..e7badc6 100644
--- a/be/src/runtime/query-state.cc
+++ b/be/src/runtime/query-state.cc
@@ -673,8 +673,8 @@ bool QueryState::ReportExecStatus() {
// without the profile so that the coordinator can still get the status and won't
// conclude that the backend has hung and cancel the query.
if (profile_buf != nullptr) {
- unique_ptr<kudu::faststring> sidecar_buf = make_unique<kudu::faststring>();
- sidecar_buf->assign_copy(profile_buf, profile_len);
+ kudu::faststring sidecar_buf;
+ sidecar_buf.assign_copy(profile_buf, profile_len);
unique_ptr<RpcSidecar> sidecar = RpcSidecar::FromFaststring(move(sidecar_buf));
int sidecar_idx;
diff --git a/be/src/util/bloom-filter-test.cc b/be/src/util/bloom-filter-test.cc
index ef25603..4936741 100644
--- a/be/src/util/bloom-filter-test.cc
+++ b/be/src/util/bloom-filter-test.cc
@@ -22,6 +22,7 @@
#include <vector>
#include "kudu/rpc/rpc_controller.h"
+#include "kudu/util/random.h"
#include "runtime/bufferpool/buffer-pool.h"
#include "runtime/bufferpool/reservation-tracker.h"
#include "runtime/mem-tracker.h"
@@ -117,10 +118,13 @@ namespace impala {
// Test that MaxNdv() and MinLogSpace() are dual
TEST(BloomFilter, MinSpaceMaxNdv) {
- for (int log2fpp = -2; log2fpp >= -63; --log2fpp) {
+ for (int log2fpp = -2; log2fpp >= -30; --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);
+ // If NO values can be added without exceeding fpp, then the space needed is
+ // trivially zero. This becomes a useless test; skip to the next iteration.
+ if (0 == derived_ndv) continue;
int derived_log_space = BloomFilter::MinLogSpace(derived_ndv, fpp);
EXPECT_EQ(derived_log_space, given_log_space) << "fpp: " << fpp
@@ -162,8 +166,8 @@ 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) { // NOLINT: loop on double
+ for (size_t ndv = 10000; ndv < 100 * 1000 * 1000; ndv *= 1.1) {
+ for (double fpp = 0.1; fpp > pow(2, -20); fpp *= 0.9) { // 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);
@@ -305,46 +309,54 @@ TEST_F(BloomFilterTest, CumulativeFind) {
// The empirical false positives we find when looking for random items is with a constant
// factor of the false positive probability the Bloom filter was constructed for.
TEST_F(BloomFilterTest, FindInvalid) {
- srand(0);
- static const int find_limit = 1 << 20;
+ // We use a deterministic pseudorandom number generator with a set seed. The reason is
+ // that with a run-dependent seed, there will always be inputs that can fail. That's a
+ // potential argument for this to be a benchmark rather than a test, although the
+ // measured quantity would be not time but deviation from predicted fpp.
+ ::kudu::Random rgen(867 + 5309);
+ static const int find_limit = 1 << 22;
unordered_set<uint32_t> to_find;
while (to_find.size() < find_limit) {
- to_find.insert(MakeRand());
+ to_find.insert(rgen.Next64());
}
static const int max_log_ndv = 19;
unordered_set<uint32_t> to_insert;
while (to_insert.size() < (1ull << max_log_ndv)) {
- const auto candidate = MakeRand();
+ const auto candidate = rgen.Next64();
if (to_find.find(candidate) == to_find.end()) {
to_insert.insert(candidate);
}
}
vector<uint32_t> shuffled_insert(to_insert.begin(), to_insert.end());
for (int log_ndv = 12; log_ndv < max_log_ndv; ++log_ndv) {
- for (int log_fpp = 4; log_fpp < 15; ++log_fpp) {
+ for (int log_fpp = 4; log_fpp < 12; ++log_fpp) {
double fpp = 1.0 / (1 << log_fpp);
const size_t ndv = 1 << log_ndv;
- const int log_bufferpool_space = BloomFilter::MinLogSpace(ndv, fpp);
- BloomFilter* bf = CreateBloomFilter(log_bufferpool_space);
+ const int log_heap_space = BloomFilter::MinLogSpace(ndv, fpp);
+ BloomFilter* bf = CreateBloomFilter(log_heap_space);
// Fill up a BF with exactly as much ndv as we planned for it:
for (size_t i = 0; i < ndv; ++i) {
- BfInsert(*bf, shuffled_insert[i]);
+ bf->Insert( shuffled_insert[i]);
}
int found = 0;
// Now we sample from the set of possible hashes, looking for hits.
for (const auto& i : to_find) {
- found += BfFind(*bf, i);
+ found += bf->Find(i);
}
EXPECT_LE(found, find_limit * fpp * 2)
- << "Too many false positives with -log2(fpp) = " << log_fpp;
+ << "Too many false positives with -log2(fpp) = " << log_fpp
+ << " and log_ndv = " << log_ndv << " and log_heap_space = " << log_heap_space;
// Because the space is rounded up to a power of 2, we might actually get a lower
// fpp than the one passed to MinLogSpace().
- const double expected_fpp =
- BloomFilter::FalsePositiveProb(ndv, log_bufferpool_space);
- EXPECT_GE(found, find_limit * expected_fpp)
- << "Too few false positives with -log2(fpp) = " << log_fpp;
- EXPECT_LE(found, find_limit * expected_fpp * 8)
- << "Too many false positives with -log2(fpp) = " << log_fpp;
+ const double expected_fpp = BloomFilter::FalsePositiveProb(ndv, log_heap_space);
+ // Fudge factors are present because filter characteristics are true in the limit,
+ // and will deviate for small samples.
+ EXPECT_GE(found, find_limit * expected_fpp * 0.75)
+ << "Too few false positives with -log2(fpp) = " << log_fpp
+ << " expected_fpp = " << expected_fpp;
+ EXPECT_LE(found, find_limit * expected_fpp * 1.25)
+ << "Too many false positives with -log2(fpp) = " << log_fpp
+ << " expected_fpp = " << expected_fpp;
}
}
}
diff --git a/be/src/util/webserver.cc b/be/src/util/webserver.cc
index 323ebaf..0ba67e0 100644
--- a/be/src/util/webserver.cc
+++ b/be/src/util/webserver.cc
@@ -205,6 +205,8 @@ string HttpStatusCodeToString(HttpStatusCode code) {
switch (code) {
case HttpStatusCode::Ok:
return "200 OK";
+ case HttpStatusCode::TemporaryRedirect:
+ return "307 Temporary Redirect";
case HttpStatusCode::BadRequest:
return "400 Bad Request";
case HttpStatusCode::AuthenticationRequired:
@@ -251,6 +253,7 @@ void SendResponse(struct sq_connection* connection, const string& response_code_
// Return the address of the remote user from the squeasel request info.
kudu::Sockaddr GetRemoteAddress(const struct sq_request_info* req) {
struct sockaddr_in addr;
+ memset(&addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
addr.sin_port = NetworkByteOrder::FromHost16(req->remote_port);
addr.sin_addr.s_addr = NetworkByteOrder::FromHost32(req->remote_ip);