You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/05/08 18:18:27 UTC
[impala] branch master updated (2fb2c7a -> 9a216f1)
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.
from 2fb2c7a [DOCS] Fix show SQL typos
new 460aef6 IMPALA-8512: Disable certain tests on Centos6
new 9a216f1 IMPALA-8517: print backtrace to debug bootstrap_toolchain
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
be/src/runtime/io/data-cache-test.cc | 18 ++++++++-----
be/src/util/filesystem-util-test.cc | 30 +++++++++++++++++----
be/src/util/filesystem-util.cc | 24 ++++++++++-------
be/src/util/filesystem-util.h | 30 ++++++++++++++-------
bin/bootstrap_toolchain.py | 48 ++++++++++++++++++---------------
tests/common/environ.py | 10 +++++++
tests/common/skip.py | 5 +++-
tests/custom_cluster/test_data_cache.py | 1 +
8 files changed, 113 insertions(+), 53 deletions(-)
[impala] 02/02: IMPALA-8517: print backtrace to debug
bootstrap_toolchain
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 9a216f1de96722a43056a724ea22071383b58360
Author: Tim Armstrong <ta...@cloudera.com>
AuthorDate: Tue May 7 11:26:13 2019 -0700
IMPALA-8517: print backtrace to debug bootstrap_toolchain
This should help track down the source of the exception if the flakiness
reoccurs.
Change-Id: Ia6205d024c67c6c70ec49e4e65967d5c91b48428
Reviewed-on: http://gerrit.cloudera.org:8080/13270
Tested-by: Tim Armstrong <ta...@cloudera.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
bin/bootstrap_toolchain.py | 48 ++++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/bin/bootstrap_toolchain.py b/bin/bootstrap_toolchain.py
index f0b54f6..07a646d 100755
--- a/bin/bootstrap_toolchain.py
+++ b/bin/bootstrap_toolchain.py
@@ -50,6 +50,7 @@ import subprocess
import sys
import tempfile
import time
+import traceback
from collections import namedtuple
@@ -412,27 +413,32 @@ def download_cdh_components(toolchain_root, cdh_components, url_prefix):
os.makedirs(cdh_components_home)
def download(component):
- pkg_directory = package_directory(cdh_components_home, component.name,
- component.version)
- if os.path.isdir(pkg_directory):
- return
-
- platform_label = ""
- # Kudu is the only component that's platform dependent.
- if component.name == "kudu":
- platform_label = "-%s" % get_platform_release_label().cdh
- # Download the package if it doesn't exist
- file_name = "{0}-{1}{2}.tar.gz".format(
- component.name, component.version, platform_label)
-
- if component.url is None:
- download_path = url_prefix + file_name
- else:
- download_path = component.url
- if "%(platform_label)" in component.url:
- download_path = \
- download_path.replace("%(platform_label)", get_platform_release_label().cdh)
- wget_and_unpack_package(download_path, file_name, cdh_components_home, False)
+ try:
+ pkg_directory = package_directory(cdh_components_home, component.name,
+ component.version)
+ if os.path.isdir(pkg_directory):
+ return
+
+ platform_label = ""
+ # Kudu is the only component that's platform dependent.
+ if component.name == "kudu":
+ platform_label = "-%s" % get_platform_release_label().cdh
+ # Download the package if it doesn't exist
+ file_name = "{0}-{1}{2}.tar.gz".format(
+ component.name, component.version, platform_label)
+
+ if component.url is None:
+ download_path = url_prefix + file_name
+ else:
+ download_path = component.url
+ if "%(platform_label)" in component.url:
+ download_path = \
+ download_path.replace("%(platform_label)", get_platform_release_label().cdh)
+ wget_and_unpack_package(download_path, file_name, cdh_components_home, False)
+ except Exception:
+ # IMPALA-8517: print backtrace to help debug flaky failures.
+ traceback.print_exc()
+ raise
execute_many(download, cdh_components)
[impala] 01/02: IMPALA-8512: Disable certain tests on Centos6
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 460aef657a7afdb0aa3bf91ac0f92c0496624379
Author: Michael Ho <kw...@cloudera.com>
AuthorDate: Mon May 6 16:45:32 2019 -0700
IMPALA-8512: Disable certain tests on Centos6
The data cache related tests rely on data cache files being created
successfully on local filesystem. The cache initialization may fail
if the cache directory resides on a ext filesystem which is affected
by KUDU-1508 (metadata corruption after hole punching in some files).
On some older versions of Centos6, the tests fail as a result of
this bug.
This change skips these tests if they detect that it's running on
an old system affected by KUDU-1508. This patch also disables a
filesystem-util test which relies on readdir() returning the correct
entries' types. On some older platforms such as Centos6, this feature
may not be fully supported on all filesystems.
Change-Id: Ifbff15415bc690f779a09ec93a7ded8b394eca10
Reviewed-on: http://gerrit.cloudera.org:8080/13271
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Tim Armstrong <ta...@cloudera.com>
---
be/src/runtime/io/data-cache-test.cc | 18 ++++++++++++------
be/src/util/filesystem-util-test.cc | 30 +++++++++++++++++++++++++-----
be/src/util/filesystem-util.cc | 24 ++++++++++++++----------
be/src/util/filesystem-util.h | 30 ++++++++++++++++++++----------
tests/common/environ.py | 10 ++++++++++
tests/common/skip.py | 5 ++++-
tests/custom_cluster/test_data_cache.py | 1 +
7 files changed, 86 insertions(+), 32 deletions(-)
diff --git a/be/src/runtime/io/data-cache-test.cc b/be/src/runtime/io/data-cache-test.cc
index cab410f..9b13918 100644
--- a/be/src/runtime/io/data-cache-test.cc
+++ b/be/src/runtime/io/data-cache-test.cc
@@ -32,18 +32,19 @@
#include "common/names.h"
-#define TEMP_BUFFER_SIZE (4096)
-#define TEST_BUFFER_SIZE (8192)
+#define BASE_CACHE_DIR "/tmp"
+#define DEFAULT_CACHE_SIZE (4 * 1024 * 1024)
+#define FNAME ("foobar")
#define NUM_TEST_DIRS (16)
#define NUM_THREADS (18)
-#define FNAME ("foobar")
#define MTIME (12345)
-#define DEFAULT_CACHE_SIZE (4 * 1024 * 1024)
+#define TEMP_BUFFER_SIZE (4096)
+#define TEST_BUFFER_SIZE (8192)
+DECLARE_bool(cache_force_single_shard);
DECLARE_int64(data_cache_file_max_size_bytes);
DECLARE_int32(data_cache_max_opened_files);
DECLARE_int32(data_cache_write_concurrency);
-DECLARE_bool(cache_force_single_shard);
namespace impala {
namespace io {
@@ -118,7 +119,7 @@ class DataCacheTest : public testing::Test {
flag_saver_.reset(new google::FlagSaver());
ASSERT_OK(test_env_->Init());
for (int i = 0; i < NUM_TEST_DIRS; ++i) {
- const string& path = Substitute("/tmp/data-cache-test.$0", i);
+ const string& path = Substitute("$0/data-cache-test.$1", BASE_CACHE_DIR, i);
ASSERT_OK(FileSystemUtil::RemoveAndCreateDirectory(path));
data_cache_dirs_.push_back(path);
}
@@ -485,5 +486,10 @@ int main(int argc, char **argv) {
int rand_seed = time(NULL);
LOG(INFO) << "rand_seed: " << rand_seed;
srand(rand_seed);
+ // Skip if the platform is affected by KUDU-1508.
+ if (!impala::FileSystemUtil::CheckForBuggyExtFS(BASE_CACHE_DIR).ok()) {
+ LOG(WARNING) << "Skipping data-cache-test due to KUDU-1508.";
+ return 0;
+ }
return RUN_ALL_TESTS();
}
diff --git a/be/src/util/filesystem-util-test.cc b/be/src/util/filesystem-util-test.cc
index 590bf7c..c8e744e 100644
--- a/be/src/util/filesystem-util-test.cc
+++ b/be/src/util/filesystem-util-test.cc
@@ -18,10 +18,13 @@
#include "filesystem-util.h"
#include <boost/filesystem.hpp>
+#include <dirent.h>
#include <sys/stat.h>
+#include <sys/types.h>
#include "common/logging.h"
#include "testutil/gtest-util.h"
+#include "util/scope-exit-trigger.h"
#include "util/test-info.h"
#include "common/names.h"
@@ -126,9 +129,29 @@ TEST(FilesystemUtil, DirEntryTypes) {
path subdir = dir / "impala-subdir";
path file = dir / "impala-file";
+ // Always cleanup on exit.
+ auto remove_dir = MakeScopeExitTrigger([&dir]() { filesystem::remove_all(dir); });
+
+ // Create the test directory and file.
ASSERT_OK(FileSystemUtil::RemoveAndCreateDirectory(subdir.string()));
ASSERT_OK(FileSystemUtil::CreateFile(file.string()));
+ // Check if the system supports listing directory entries' types for 'impala-dir'.
+ // Some filesystem may not have full support for it on older platforms.
+ DIR* dir_stream = opendir(dir.c_str());
+ ASSERT_TRUE(dir_stream != nullptr);
+ auto close_dir = MakeScopeExitTrigger([&dir_stream]() { closedir(dir_stream); });
+ const dirent* dir_entry;
+ while ((dir_entry = readdir(dir_stream)) != nullptr) {
+ const char* entry_name = dir_entry->d_name;
+ if ((strcmp(entry_name, "impala-subdir") == 0 && dir_entry->d_type != DT_DIR) ||
+ (strcmp(entry_name, "impala-file") == 0 && dir_entry->d_type != DT_REG)) {
+ LOG(WARNING) << Substitute("readdir() failed to list directory entry types of $0. "
+ "Skipping DirEntryType test.", dir.string());
+ return;
+ }
+ }
+
// Verify that all directory entires are listed with the default parameters.
vector<string> entries;
ASSERT_OK(FileSystemUtil::Directory::GetEntryNames(dir.string(), &entries));
@@ -138,19 +161,16 @@ TEST(FilesystemUtil, DirEntryTypes) {
}
// Verify that only directory type entries are listed with DIR_ENTRY_DIR.
- entries.resize(0);
+ entries.clear();
ASSERT_OK(FileSystemUtil::Directory::GetEntryNames(dir.string(), &entries, 0,
FileSystemUtil::Directory::DIR_ENTRY_DIR));
ASSERT_EQ(entries.size(), 1);
EXPECT_TRUE(entries[0] == "impala-subdir");
// Verify that only file type entries are listed with DIR_ENTRY_REG.
- entries.resize(0);
+ entries.clear();
ASSERT_OK(FileSystemUtil::Directory::GetEntryNames(dir.string(), &entries, 0,
FileSystemUtil::Directory::DIR_ENTRY_REG));
ASSERT_EQ(entries.size(), 1);
EXPECT_TRUE(entries[0] == "impala-file");
-
- // Cleanup.
- filesystem::remove_all(dir);
}
diff --git a/be/src/util/filesystem-util.cc b/be/src/util/filesystem-util.cc
index 13775a9..bc9d7fd 100644
--- a/be/src/util/filesystem-util.cc
+++ b/be/src/util/filesystem-util.cc
@@ -305,7 +305,8 @@ Status FileSystemUtil::Directory::GetEntryNames(const string& path,
}
// Copied and pasted from Kudu source: src/kudu/fs/log_block_manager.cc
-bool FileSystemUtil::IsBuggyEl6Kernel(const string& kernel_release) {
+bool FileSystemUtil::IsBuggyEl6Kernel() {
+ const string& kernel_release = kudu::Env::Default()->GetKernelRelease();
autodigit_less lt;
// Only el6 is buggy.
@@ -324,17 +325,20 @@ bool FileSystemUtil::IsBuggyEl6Kernel(const string& kernel_release) {
return lt(kernel_release, "2.6.32-674");
}
-Status FileSystemUtil::CheckHolePunch(const string& path) {
- kudu::Env* env = kudu::Env::Default();
-
- // Check if the filesystem of 'path' is vulnerable to to KUDU-1508.
+Status FileSystemUtil::CheckForBuggyExtFS(const string& path) {
bool is_on_ext;
- KUDU_RETURN_IF_ERROR(env->IsOnExtFilesystem(path, &is_on_ext),
+ KUDU_RETURN_IF_ERROR(kudu::Env::Default()->IsOnExtFilesystem(path, &is_on_ext),
Substitute("Failed to check filesystem type at $0", path));
- if (is_on_ext && IsBuggyEl6Kernel(env->GetKernelRelease())) {
- return Status(Substitute("Data dir $0 is on an ext4 filesystem vulnerable to "
+ if (is_on_ext && IsBuggyEl6Kernel()) {
+ return Status(Substitute("Data dir $0 is on an ext filesystem which is affected by "
"KUDU-1508.", path));
}
+ return Status::OK();
+}
+
+Status FileSystemUtil::CheckHolePunch(const string& path) {
+ // Check if the filesystem of 'path' is affected by KUDU-1508.
+ RETURN_IF_ERROR(CheckForBuggyExtFS(path));
// Open the test file.
string filename = JoinPathSegments(path, PrintId(GenerateUUID()));
@@ -358,7 +362,7 @@ Status FileSystemUtil::CheckHolePunch(const string& path) {
const off_t init_file_size = buffer_size * 4;
uint64_t sz;
- KUDU_RETURN_IF_ERROR(env->GetFileSizeOnDisk(filename, &sz),
+ KUDU_RETURN_IF_ERROR(kudu::Env::Default()->GetFileSizeOnDisk(filename, &sz),
"Failed to get pre-punch file size");
if (sz != init_file_size) {
return Status(Substitute("Unexpected pre-punch file size for $0: expected $1 but "
@@ -372,7 +376,7 @@ Status FileSystemUtil::CheckHolePunch(const string& path) {
"Failed to punch hole");
const int final_file_size = init_file_size - hole_size;
- KUDU_RETURN_IF_ERROR(env->GetFileSizeOnDisk(filename, &sz),
+ KUDU_RETURN_IF_ERROR(kudu::Env::Default()->GetFileSizeOnDisk(filename, &sz),
"Failed to get post-punch file size");
if (sz != final_file_size) {
return Status(Substitute("Unexpected post-punch file size for $0: expected $1 but "
diff --git a/be/src/util/filesystem-util.h b/be/src/util/filesystem-util.h
index b354f5f..5471baa 100644
--- a/be/src/util/filesystem-util.h
+++ b/be/src/util/filesystem-util.h
@@ -85,11 +85,12 @@ class FileSystemUtil {
static bool GetRelativePath(const std::string& path, const std::string& start,
std::string* relpath);
- /// Ext4 on certain El6 releases may result in inconsistent metadata after punching
- /// holes in files. The filesystem may require fsck repair on next reboot. See KUDU-1508
- /// for details. This function returns true iff the kernel version Impala is running on
- /// is vulnerable to KUDU-1508.
- static bool IsBuggyEl6Kernel(const string& kernel_release);
+ /// Ext filesystem on certain kernel versions may result in inconsistent metadata after
+ /// punching holes in files. The filesystem may require fsck repair on next reboot.
+ /// See KUDU-1508 for details. This function checks if the filesystem at 'path' resides
+ /// in a ext filesystem and the kernel version is affected by KUDU-1058. If so, return
+ /// error status; Returns OK otherwise.
+ static Status CheckForBuggyExtFS(const std::string& path);
/// Checks if the filesystem at the directory 'path' supports hole punching (i.e.
/// calling fallocate with FALLOC_FL_PUNCH_HOLE).
@@ -102,7 +103,7 @@ class FileSystemUtil {
/// - reading the test file's size failed.
///
/// Returns OK otherwise.
- static Status CheckHolePunch(const string& path);
+ static Status CheckHolePunch(const std::string& path);
class Directory {
public:
@@ -122,9 +123,10 @@ class FileSystemUtil {
~Directory();
/// Reads the next directory entry and sets 'entry_name' to the entry name.
- /// Returns 'false' if an error occured or no more entries were found in the
- /// directory. If 'type' is specified, only entries of 'type' will be included.
- /// Return 'true' on success.
+ /// Returns false if an error occured or no more entries were found in the directory.
+ /// If 'type' is specified and filesystem supports returning the types of directory
+ /// entries, only entries of 'type' will be included. Otherwise, it may return
+ /// entries of all types. Return 'true' on success.
bool GetNextEntryName(std::string* entry_name, EntryType type = DIR_ENTRY_ANY);
/// Returns the status of the previous directory operation.
@@ -133,7 +135,9 @@ class FileSystemUtil {
/// Reads no more than 'max_result_size' directory entries from 'path' and returns
/// their names in 'entry_names' vector. If 'max_result_size' <= 0, every directory
/// entry is returned. Directory entries "." and ".." will be skipped. If 'type' is
- /// specified, only entries of 'type' will be included.
+ /// specified and filesystem of 'path' supports returning type of directory entries,
+ /// only entries of 'type' will be included in 'entry_names'. Otherwise, it will
+ /// include entries of all types.
static Status GetEntryNames(const string& path, std::vector<std::string>* entry_names,
int max_result_size = 0, EntryType type = DIR_ENTRY_ANY);
@@ -146,6 +150,12 @@ class FileSystemUtil {
Directory(const Directory&);
Directory& operator=(const Directory&);
};
+
+ private:
+
+ /// This function returns true iff the kernel version Impala is running on
+ /// is affected by KUDU-1508.
+ static bool IsBuggyEl6Kernel();
};
}
diff --git a/tests/common/environ.py b/tests/common/environ.py
index f5df0a1..6e032f5 100644
--- a/tests/common/environ.py
+++ b/tests/common/environ.py
@@ -49,6 +49,16 @@ IS_DOCKERIZED_TEST_CLUSTER = docker_network is not None
impalad_basedir = \
os.path.realpath(os.path.join(IMPALA_HOME, 'be/build', build_type_dir)).rstrip('/')
+# Detects if the platform is a version of Centos6 which may be affected by KUDU-1508.
+# Default to the minimum kernel version which isn't affected by KUDU-1508 and parses
+# the output of `uname -a` for the actual kernel version.
+kernel_version = [2, 6, 32, 674]
+kernel_release = os.uname()[2]
+kernel_version_regex = re.compile(r'(\d+)\.(\d+)\.(\d+)\-(\d+).*')
+kernel_version_match = kernel_version_regex.match(kernel_release)
+if kernel_version_match is not None and len(kernel_version_match.groups()) == 4:
+ kernel_version = map(lambda x: int(x), list(kernel_version_match.groups()))
+IS_BUGGY_EL6_KERNEL = 'el6' in kernel_release and kernel_version < [2, 6, 32, 674]
class ImpalaBuildFlavors:
"""
diff --git a/tests/common/skip.py b/tests/common/skip.py
index 646695b..3720378 100644
--- a/tests/common/skip.py
+++ b/tests/common/skip.py
@@ -25,7 +25,7 @@ import pytest
from functools import partial
from tests.common.environ import (IMPALA_TEST_CLUSTER_PROPERTIES,
- IS_DOCKERIZED_TEST_CLUSTER)
+ IS_DOCKERIZED_TEST_CLUSTER, IS_BUGGY_EL6_KERNEL)
from tests.common.kudu_test_suite import get_kudu_master_flag
from tests.util.filesystem_utils import (
IS_ABFS,
@@ -118,6 +118,9 @@ class SkipIf:
not_ec = pytest.mark.skipif(not IS_EC, reason="Erasure Coding needed")
no_secondary_fs = pytest.mark.skipif(not SECONDARY_FILESYSTEM,
reason="Secondary filesystem needed")
+ is_buggy_el6_kernel = pytest.mark.skipif(
+ IS_BUGGY_EL6_KERNEL, reason="Kernel is affected by KUDU-1508")
+
class SkipIfIsilon:
caching = pytest.mark.skipif(IS_ISILON, reason="SET CACHED not implemented for Isilon")
diff --git a/tests/custom_cluster/test_data_cache.py b/tests/custom_cluster/test_data_cache.py
index 88464bb..c774189 100644
--- a/tests/custom_cluster/test_data_cache.py
+++ b/tests/custom_cluster/test_data_cache.py
@@ -22,6 +22,7 @@ from tests.common.skip import SkipIf, SkipIfEC
@SkipIf.not_hdfs
+@SkipIf.is_buggy_el6_kernel
@SkipIfEC.scheduling
class TestDataCache(CustomClusterTestSuite):
""" This test enables the data cache and verfies that cache hit and miss counts