IMPALA-3569: handle errors in timezone db initialization Change-Id: I6b4d5e6b992ea023f801edb7b487e57f39920c03 Reviewed-on: http://gerrit.cloudera.org:8080/3125 Reviewed-by: Dan Hecht <[email protected]> Tested-by: Internal Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/1ccfc45d Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/1ccfc45d Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/1ccfc45d Branch: refs/heads/master Commit: 1ccfc45d41acabd5fc9330a9a8b4bb50284c3e99 Parents: 51869ea Author: Tim Armstrong <[email protected]> Authored: Wed May 18 12:05:53 2016 -0700 Committer: Tim Armstrong <[email protected]> Committed: Mon May 23 08:40:19 2016 -0700 ---------------------------------------------------------------------- be/src/common/init.cc | 2 ++ be/src/exprs/expr-test.cc | 1 + be/src/exprs/timestamp-functions-ir.cc | 44 +++-------------------------- be/src/exprs/timestamp-functions.h | 6 ++-- be/src/exprs/timezone_db.cc | 32 +++++++++++++++++++++ be/src/runtime/exec-env.cc | 2 -- be/src/runtime/exec-env.h | 2 -- 7 files changed, 42 insertions(+), 47 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1ccfc45d/be/src/common/init.cc ---------------------------------------------------------------------- diff --git a/be/src/common/init.cc b/be/src/common/init.cc index 9a47c1c..b116059 100644 --- a/be/src/common/init.cc +++ b/be/src/common/init.cc @@ -21,6 +21,7 @@ #include "common/status.h" #include "exec/kudu-util.h" #include "exprs/expr.h" +#include "exprs/timestamp-functions.h" #include "gutil/atomicops.h" #include "util/cpu-info.h" #include "util/debug-util.h" @@ -187,6 +188,7 @@ void impala::InitCommonRuntime(int argc, char** argv, bool init_jvm, AtomicOps_x86CPUFeaturesInit(); impala::InitThreading(); impala::TimestampParser::Init(); + ABORT_IF_ERROR(impala::TimezoneDatabase::Initialize()); ABORT_IF_ERROR(impala::InitAuth(argv[0])); // Initialize maintenance_thread after InitGoogleLoggingSafe and InitThreading. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1ccfc45d/be/src/exprs/expr-test.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc index 349946a..09f60cb 100644 --- a/be/src/exprs/expr-test.cc +++ b/be/src/exprs/expr-test.cc @@ -34,6 +34,7 @@ #include "exprs/like-predicate.h" #include "exprs/literal.h" #include "exprs/null-literal.h" +#include "exprs/timestamp-functions.h" #include "gen-cpp/Exprs_types.h" #include "gen-cpp/hive_metastore_types.h" #include "rpc/thrift-client.h" http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1ccfc45d/be/src/exprs/timestamp-functions-ir.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/timestamp-functions-ir.cc b/be/src/exprs/timestamp-functions-ir.cc index 9808419..08992d9 100644 --- a/be/src/exprs/timestamp-functions-ir.cc +++ b/be/src/exprs/timestamp-functions-ir.cc @@ -33,8 +33,6 @@ #include "udf/udf-internal.h" #include "runtime/runtime-state.h" -#define TIMEZONE_DATABASE "be/files/date_time_zonespec.csv" - #include "common/names.h" using boost::algorithm::iequals; @@ -770,8 +768,8 @@ TimestampVal TimestampFunctions::FromUtc(FunctionContext* context, if (!ts_value.HasDateOrTime()) return TimestampVal::null(); const StringValue& tz_string_value = StringValue::FromStringVal(tz_string_val); - time_zone_ptr timezone = - TimezoneDatabase::FindTimezone(tz_string_value.DebugString(), ts_value); + time_zone_ptr timezone = TimezoneDatabase::FindTimezone( + string(tz_string_value.ptr, tz_string_value.len), ts_value); if (timezone == NULL) { // This should return null. Hive just ignores it. stringstream ss; @@ -799,8 +797,8 @@ TimestampVal TimestampFunctions::ToUtc(FunctionContext* context, if (!ts_value.HasDateOrTime()) return TimestampVal::null(); const StringValue& tz_string_value = StringValue::FromStringVal(tz_string_val); - time_zone_ptr timezone = - TimezoneDatabase::FindTimezone(tz_string_value.DebugString(), ts_value); + time_zone_ptr timezone = TimezoneDatabase::FindTimezone( + string(tz_string_value.ptr, tz_string_value.len), ts_value); // This should raise some sort of error or at least null. Hive Just ignores it. if (timezone == NULL) { stringstream ss; @@ -817,40 +815,6 @@ TimestampVal TimestampFunctions::ToUtc(FunctionContext* context, return return_val; } -TimezoneDatabase::TimezoneDatabase() { - // Create a temporary file and write the timezone information. The boost - // interface only loads this format from a file. We don't want to raise - // an error here since this is done when the backend is created and this - // information might not actually get used by any queries. - char filestr[] = "/tmp/impala.tzdb.XXXXXXX"; - FILE* file; - int fd; - if ((fd = mkstemp(filestr)) == -1) { - LOG(ERROR) << "Could not create temporary timezone file: " << filestr; - return; - } - if ((file = fopen(filestr, "w")) == NULL) { - unlink(filestr); - close(fd); - LOG(ERROR) << "Could not open temporary timezone file: " << filestr; - return; - } - if (fputs(TIMEZONE_DATABASE_STR, file) == EOF) { - unlink(filestr); - close(fd); - fclose(file); - LOG(ERROR) << "Could not load temporary timezone file: " << filestr; - return; - } - fclose(file); - tz_database_.load_from_file(string(filestr)); - tz_region_list_ = tz_database_.region_list(); - unlink(filestr); - close(fd); -} - -TimezoneDatabase::~TimezoneDatabase() { } - time_zone_ptr TimezoneDatabase::FindTimezone(const string& tz, const TimestampValue& tv) { // The backing database does not capture some subtleties, there are special cases if ((tv.date().year() < 2011 || (tv.date().year() == 2011 && tv.date().month() < 4)) && http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1ccfc45d/be/src/exprs/timestamp-functions.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/timestamp-functions.h b/be/src/exprs/timestamp-functions.h index 3e08eb3..23c0d10 100644 --- a/be/src/exprs/timestamp-functions.h +++ b/be/src/exprs/timestamp-functions.h @@ -17,8 +17,8 @@ #define IMPALA_EXPRS_TIMESTAMP_FUNCTIONS_H #include <boost/date_time/local_time/local_time.hpp> -#include <string> +#include "common/status.h" #include "udf/udf.h" using namespace impala_udf; @@ -186,8 +186,8 @@ class TimestampFunctions { /// Functions to load and access the timestamp database. class TimezoneDatabase { public: - TimezoneDatabase(); - ~TimezoneDatabase(); + /// Set up the static timezone database. + static Status Initialize(); /// Converts the name of a timezone to a boost timezone object. /// Some countries change their timezones, the tiemstamp is required to correctly http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1ccfc45d/be/src/exprs/timezone_db.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/timezone_db.cc b/be/src/exprs/timezone_db.cc index e7c4301..b45ec73 100644 --- a/be/src/exprs/timezone_db.cc +++ b/be/src/exprs/timezone_db.cc @@ -14,11 +14,14 @@ #include "timestamp-functions.h" +#include "gutil/strings/substitute.h" + #include "common/names.h" using boost::local_time::tz_database; using boost::local_time::time_zone_ptr; using boost::local_time::posix_time_zone; +using strings::Substitute; namespace impala { @@ -653,5 +656,34 @@ const char* TimezoneDatabase::TIMEZONE_DATABASE_STR = "\"ID\",\"STD ABBR\",\"STD \"WET\",\"WET\",\"Western European Time\",\"WEST\",\"Western European Summer Time\",\"+00:00:00\",\"+01:00:00\",\"-1;0;3\",\"+01:00:00\",\"-1;0;10\",\"+01:00:00\"\n\ \"Zulu\",\"UTC\",\"Coordinated Universal Time\",\"\",\"\",\"+00:00:00\",\"+00:00:00\",\"\",\"\",\"\",\"\""; +Status TimezoneDatabase::Initialize() { + // Create a temporary file and write the timezone information. The boost + // interface only loads this format from a file. We don't want to raise + // an error here since this is done when the backend is created and this + // information might not actually get used by any queries. + char filestr[] = "/tmp/impala.tzdb.XXXXXXX"; + FILE* file; + int fd; + if ((fd = mkstemp(filestr)) == -1) { + return Status(Substitute("Could not create temporary timezone file: $0", filestr)); + } + if ((file = fopen(filestr, "w")) == NULL) { + unlink(filestr); + close(fd); + return Status(Substitute("Could not open temporary timezone file: $0", filestr)); + } + if (fputs(TIMEZONE_DATABASE_STR, file) == EOF) { + unlink(filestr); + close(fd); + fclose(file); + return Status(Substitute("Could not load temporary timezone file: $0", filestr)); + } + fclose(file); + tz_database_.load_from_file(string(filestr)); + tz_region_list_ = tz_database_.region_list(); + unlink(filestr); + close(fd); + return Status::OK(); +} } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1ccfc45d/be/src/runtime/exec-env.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/exec-env.cc b/be/src/runtime/exec-env.cc index 046c049..07378e4 100644 --- a/be/src/runtime/exec-env.cc +++ b/be/src/runtime/exec-env.cc @@ -164,7 +164,6 @@ ExecEnv::ExecEnv() FLAGS_coordinator_rpc_threads, numeric_limits<int32_t>::max())), async_rpc_pool_(new CallableThreadPool("rpc-pool", "async-rpc-sender", 8, 10000)), enable_webserver_(FLAGS_enable_webserver), - tz_database_(TimezoneDatabase()), is_fe_tests_(false), backend_address_(MakeNetworkAddress(FLAGS_hostname, FLAGS_be_port)), is_pseudo_distributed_llama_(false) { @@ -216,7 +215,6 @@ ExecEnv::ExecEnv(const string& hostname, int backend_port, int subscriber_port, FLAGS_coordinator_rpc_threads, numeric_limits<int32_t>::max())), async_rpc_pool_(new CallableThreadPool("rpc-pool", "async-rpc-sender", 8, 10000)), enable_webserver_(FLAGS_enable_webserver && webserver_port > 0), - tz_database_(TimezoneDatabase()), is_fe_tests_(false), backend_address_(MakeNetworkAddress(FLAGS_hostname, FLAGS_be_port)), is_pseudo_distributed_llama_(false) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1ccfc45d/be/src/runtime/exec-env.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/exec-env.h b/be/src/runtime/exec-env.h index c52c68b..e405bf8 100644 --- a/be/src/runtime/exec-env.h +++ b/be/src/runtime/exec-env.h @@ -21,7 +21,6 @@ #include <boost/thread/thread.hpp> #include "common/status.h" -#include "exprs/timestamp-functions.h" #include "runtime/backend-client.h" #include "util/cgroups-mgr.h" #include "util/hdfs-bulk-ops.h" // For declaration of HdfsOpThreadPool @@ -154,7 +153,6 @@ class ExecEnv { private: static ExecEnv* exec_env_; - TimezoneDatabase tz_database_; bool is_fe_tests_; /// Address of the Impala backend server instance
