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 2017/10/24 22:21:00 UTC
[1/4] incubator-impala git commit: IMPALA-6060: Check the return
value of JNI exception handling functions
Repository: incubator-impala
Updated Branches:
refs/heads/master 308bd5d58 -> 2bd010781
IMPALA-6060: Check the return value of JNI exception handling functions
When JVM runs out of memory and throws an error to JNI, the error
handling code uses JNI to get the exception message, resulting in a
null pointer and crashing the process. This patch adds error handling
code to JniUtil::GetJniExceptionMsg().
Change-Id: Ie3ed88bf8739c56a066f2402727c8204e96aa116
Reviewed-on: http://gerrit.cloudera.org:8080/8334
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public 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/31c440bf
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/31c440bf
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/31c440bf
Branch: refs/heads/master
Commit: 31c440bf341c18a838e15360bb4b2e8c90f86c75
Parents: 308bd5d
Author: Tianyi Wang <tw...@cloudera.com>
Authored: Wed Oct 18 15:54:14 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Oct 24 21:22:58 2017 +0000
----------------------------------------------------------------------
be/src/util/jni-util.cc | 23 ++++++++++++++++----
be/src/util/jni-util.h | 4 ++--
be/src/util/logging-support.cc | 1 +
.../java/org/apache/impala/common/JniUtil.java | 20 +++++++----------
4 files changed, 30 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/31c440bf/be/src/util/jni-util.cc
----------------------------------------------------------------------
diff --git a/be/src/util/jni-util.cc b/be/src/util/jni-util.cc
index 220eb83..3128697 100644
--- a/be/src/util/jni-util.cc
+++ b/be/src/util/jni-util.cc
@@ -27,7 +27,8 @@
namespace impala {
-Status JniUtfCharGuard::create(JNIEnv *env, jstring jstr, JniUtfCharGuard *out) {
+Status JniUtfCharGuard::create(JNIEnv* env, jstring jstr, JniUtfCharGuard* out) {
+ DCHECK(jstr != nullptr);
DCHECK(!env->ExceptionCheck());
const char* utf_chars = env->GetStringUTFChars(jstr, nullptr);
bool exception_check = static_cast<bool>(env->ExceptionCheck());
@@ -180,17 +181,31 @@ void JniUtil::InitLibhdfs() {
}
Status JniUtil::GetJniExceptionMsg(JNIEnv* env, bool log_stack, const string& prefix) {
- jthrowable exc = (env)->ExceptionOccurred();
+ jthrowable exc = env->ExceptionOccurred();
if (exc == nullptr) return Status::OK();
env->ExceptionClear();
DCHECK(throwable_to_string_id() != nullptr);
- auto msg = static_cast<jstring>(env->CallStaticObjectMethod(jni_util_class(),
+ const char* oom_msg_template = "$0 threw an unchecked exception. The JVM is likely out "
+ "of memory (OOM).";
+ jstring msg = static_cast<jstring>(env->CallStaticObjectMethod(jni_util_class(),
throwable_to_string_id(), exc));
+ if (env->ExceptionOccurred()) {
+ env->ExceptionClear();
+ string oom_msg = Substitute(oom_msg_template, "throwableToString");
+ LOG(ERROR) << oom_msg;
+ return Status(oom_msg);
+ }
JniUtfCharGuard msg_str_guard;
RETURN_IF_ERROR(JniUtfCharGuard::create(env, msg, &msg_str_guard));
if (log_stack) {
- auto stack = static_cast<jstring>(env->CallStaticObjectMethod(jni_util_class(),
+ jstring stack = static_cast<jstring>(env->CallStaticObjectMethod(jni_util_class(),
throwable_to_stack_trace_id(), exc));
+ if (env->ExceptionOccurred()) {
+ env->ExceptionClear();
+ string oom_msg = Substitute(oom_msg_template, "throwableToStackTrace");
+ LOG(ERROR) << oom_msg;
+ return Status(oom_msg);
+ }
JniUtfCharGuard c_stack_guard;
RETURN_IF_ERROR(JniUtfCharGuard::create(env, stack, &c_stack_guard));
VLOG(1) << c_stack_guard.get();
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/31c440bf/be/src/util/jni-util.h
----------------------------------------------------------------------
diff --git a/be/src/util/jni-util.h b/be/src/util/jni-util.h
index 77abb4d..43ac131 100644
--- a/be/src/util/jni-util.h
+++ b/be/src/util/jni-util.h
@@ -154,9 +154,9 @@ class JniUtfCharGuard {
if (utf_chars != nullptr) env->ReleaseStringUTFChars(jstr, utf_chars);
}
- /// Try to get chars from jstring. If error is returned, utf_chars and get() remain
+ /// Try to get chars from jstr. If error is returned, utf_chars and get() remain
/// to be nullptr, otherwise they point to a valid char sequence. The char sequence
- /// lives as long as this guard.
+ /// lives as long as this guard. jstr should not be null.
static Status create(JNIEnv* env, jstring jstr, JniUtfCharGuard* out);
/// Get the char sequence. Returns nullptr if the guard does hold a char sequence.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/31c440bf/be/src/util/logging-support.cc
----------------------------------------------------------------------
diff --git a/be/src/util/logging-support.cc b/be/src/util/logging-support.cc
index 3d90ed6..400bd67 100644
--- a/be/src/util/logging-support.cc
+++ b/be/src/util/logging-support.cc
@@ -42,6 +42,7 @@ JNIEXPORT void JNICALL
Java_org_apache_impala_util_NativeLogger_Log(
JNIEnv* env, jclass caller_class, int severity, jstring msg, jstring file,
int line_number) {
+ DCHECK(file != nullptr);
JniUtfCharGuard filename_guard;
RETURN_VOID_IF_ERROR(JniUtfCharGuard::create(env, file, &filename_guard));
JniUtfCharGuard msg_guard;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/31c440bf/fe/src/main/java/org/apache/impala/common/JniUtil.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/common/JniUtil.java b/fe/src/main/java/org/apache/impala/common/JniUtil.java
index 1fb00c8..6aec0d4 100644
--- a/fe/src/main/java/org/apache/impala/common/JniUtil.java
+++ b/fe/src/main/java/org/apache/impala/common/JniUtil.java
@@ -60,18 +60,14 @@ public class JniUtil {
* the chain of causes each in a separate line.
*/
public static String throwableToString(Throwable t) {
- Writer output = new StringWriter();
- try {
- output.write(String.format("%s: %s", t.getClass().getSimpleName(),
- t.getMessage()));
- // Follow the chain of exception causes and print them as well.
- Throwable cause = t;
- while ((cause = cause.getCause()) != null) {
- output.write(String.format("\nCAUSED BY: %s: %s",
- cause.getClass().getSimpleName(), cause.getMessage()));
- }
- } catch (IOException e) {
- throw new Error(e);
+ StringWriter output = new StringWriter();
+ output.write(String.format("%s: %s", t.getClass().getSimpleName(),
+ t.getMessage()));
+ // Follow the chain of exception causes and print them as well.
+ Throwable cause = t;
+ while ((cause = cause.getCause()) != null) {
+ output.write(String.format("\nCAUSED BY: %s: %s",
+ cause.getClass().getSimpleName(), cause.getMessage()));
}
return output.toString();
}
[4/4] incubator-impala git commit: IMPALA-6099: Fix crash in
CheckForAlwaysFalse()
Posted by ta...@apache.org.
IMPALA-6099: Fix crash in CheckForAlwaysFalse()
Filter states indexed by FilterStats::ROW_GROUPS_KEY was only relevant
if the runtime filter is bound by partitioning columns. This is no
longer true since always-false filter can filter out row groups as
well, which results in a crash in CheckForAlwaysFalse(). This patch
registers row groups counters unconditionally to fix this bug.
Change-Id: I3eda43845b78516c1e76e07e0d2dd9d862168e1d
Reviewed-on: http://gerrit.cloudera.org:8080/8369
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public 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/2bd01078
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/2bd01078
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/2bd01078
Branch: refs/heads/master
Commit: 2bd010781c07fee9dc09924856ac7ced11e6be3e
Parents: c87ad36
Author: Tianyi Wang <tw...@cloudera.com>
Authored: Mon Oct 23 13:38:42 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Oct 24 22:19:22 2017 +0000
----------------------------------------------------------------------
be/src/exec/filter-context.cc | 4 ++--
be/src/exec/filter-context.h | 2 +-
be/src/exec/scan-node.cc | 4 +---
3 files changed, 4 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2bd01078/be/src/exec/filter-context.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/filter-context.cc b/be/src/exec/filter-context.cc
index 61e104f..2a07c6f 100644
--- a/be/src/exec/filter-context.cc
+++ b/be/src/exec/filter-context.cc
@@ -33,14 +33,14 @@ const std::string FilterStats::ROWS_KEY = "Rows";
const char* FilterContext::LLVM_CLASS_NAME = "struct.impala::FilterContext";
-FilterStats::FilterStats(RuntimeProfile* runtime_profile, bool is_partition_filter) {
+FilterStats::FilterStats(RuntimeProfile* runtime_profile) {
DCHECK(runtime_profile != nullptr);
profile = runtime_profile;
RegisterCounterGroup(FilterStats::SPLITS_KEY);
RegisterCounterGroup(FilterStats::FILES_KEY);
// TODO: These only apply to Parquet, so only register them in that case.
RegisterCounterGroup(FilterStats::ROWS_KEY);
- if (is_partition_filter) RegisterCounterGroup(FilterStats::ROW_GROUPS_KEY);
+ RegisterCounterGroup(FilterStats::ROW_GROUPS_KEY);
}
void FilterStats::IncrCounters(const string& key, int32_t total, int32_t processed,
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2bd01078/be/src/exec/filter-context.h
----------------------------------------------------------------------
diff --git a/be/src/exec/filter-context.h b/be/src/exec/filter-context.h
index dcd2a94..0806fd3 100644
--- a/be/src/exec/filter-context.h
+++ b/be/src/exec/filter-context.h
@@ -41,7 +41,7 @@ class FilterStats {
/// Constructs a new FilterStats object with a profile that is a child of
/// 'profile'. 'is_partition_filter' determines whether partition-level counters are
/// registered.
- FilterStats(RuntimeProfile* runtime_profile, bool is_partition_filter);
+ FilterStats(RuntimeProfile* runtime_profile);
static const std::string ROW_GROUPS_KEY;
static const std::string FILES_KEY;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2bd01078/be/src/exec/scan-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/scan-node.cc b/be/src/exec/scan-node.cc
index eba7727..fd2453d 100644
--- a/be/src/exec/scan-node.cc
+++ b/be/src/exec/scan-node.cc
@@ -57,7 +57,6 @@ const string ScanNode::NUM_SCANNER_THREADS_STARTED =
Status ScanNode::Init(const TPlanNode& tnode, RuntimeState* state) {
RETURN_IF_ERROR(ExecNode::Init(tnode, state));
-
const TQueryOptions& query_options = state->query_options();
for (const TRuntimeFilterDesc& filter_desc : tnode.runtime_filters) {
auto it = filter_desc.planid_to_target_ndx.find(tnode.node_id);
@@ -85,8 +84,7 @@ Status ScanNode::Init(const TPlanNode& tnode, RuntimeState* state) {
RuntimeProfile* profile =
RuntimeProfile::Create(state->obj_pool(), filter_profile_title);
runtime_profile_->AddChild(profile);
- filter_ctx.stats = state->obj_pool()->Add(new FilterStats(profile,
- target.is_bound_by_partition_columns));
+ filter_ctx.stats = state->obj_pool()->Add(new FilterStats(profile));
}
return Status::OK();
[3/4] incubator-impala git commit: IMPALA-6076: Parquet BIT_PACKED
deprecation warning
Posted by ta...@apache.org.
IMPALA-6076: Parquet BIT_PACKED deprecation warning
Every 100th time that we open a Parquet column with the
deprecated BIT_PACKED encoding, an error is logged. We do this
per-column instead of per-file because Impala historically
listed the BIT_PACKED encoding in file metadata even when it
wasn't used for any columns - see IMPALA-5636.
Testing:
Manually tested by running a query repeatedly against a
BIT_PACKED sample file (which I created for my IMPALA-4177
patch). Ran "tail -f logs/cluster/impalad.WARNING" and checked
that the warning was logged periodically.
Change-Id: I02dd4009089a264b28376492b1b40361d767d5d9
Reviewed-on: http://gerrit.cloudera.org:8080/8370
Reviewed-by: Lars Volker <lv...@cloudera.com>
Tested-by: Impala Public 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/c87ad363
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c87ad363
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c87ad363
Branch: refs/heads/master
Commit: c87ad3631a4f3f1854759937ae0f8de63cb6e5dc
Parents: 1640aa9
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Tue Oct 24 10:31:24 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Oct 24 22:11:39 2017 +0000
----------------------------------------------------------------------
be/src/exec/parquet-column-readers.cc | 7 +++++++
1 file changed, 7 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c87ad363/be/src/exec/parquet-column-readers.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/parquet-column-readers.cc b/be/src/exec/parquet-column-readers.cc
index ad12916..6d211a6 100644
--- a/be/src/exec/parquet-column-readers.cc
+++ b/be/src/exec/parquet-column-readers.cc
@@ -46,6 +46,9 @@ DEFINE_bool(convert_legacy_hive_parquet_utc_timestamps, false,
"When true, TIMESTAMPs read from files written by Parquet-MR (used by Hive) will "
"be converted from UTC to local time. Writes are unaffected.");
+// Throttle deprecation warnings to - only print warning with this frequency.
+static const int BITPACKED_DEPRECATION_WARNING_FREQUENCY = 100;
+
// Max data page header size in bytes. This is an estimate and only needs to be an upper
// bound. It is theoretically possible to have a page header of any size due to string
// value statistics, but in practice we'll have trouble reading string values this large.
@@ -100,6 +103,10 @@ Status ParquetLevelDecoder::Init(const string& filename,
case parquet::Encoding::BIT_PACKED:
num_bytes = BitUtil::Ceil(num_buffered_values, 8);
bit_reader_.Reset(*data, num_bytes);
+ LOG_EVERY_N(WARNING, BITPACKED_DEPRECATION_WARNING_FREQUENCY)
+ << filename << " uses deprecated Parquet BIT_PACKED encoding for rep or def "
+ << "levels. This will be removed in the future - see IMPALA-6077. Warning "
+ << "every " << BITPACKED_DEPRECATION_WARNING_FREQUENCY << " occurrences.";
break;
default: {
stringstream ss;
[2/4] incubator-impala git commit: IMPALA-5599: Clean up references
to TimestampValue in be/src.
Posted by ta...@apache.org.
IMPALA-5599: Clean up references to TimestampValue in be/src.
This is a follow-on commit to d53f43b4. In this patch we replace
all inappropriate usage of TimestampValue in be/src with simpler
data types for Unix timestamps, and where conversions to strings
are needed, we use the interfaces introduced by the previous
commit.
TimestampValue::{LocalTime(), UtcTime()} have been retired.
BE, FE, EE, JDBC, and Cluster Tests were run to check for
regressions. All the tests passed. Also manually inspected query
profiles to check that datestamps and time intervals are sane.
Change-Id: I642a1d713597826bb7c15cd2ecb6638cb813a02c
Reviewed-on: http://gerrit.cloudera.org:8080/8305
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Impala Public 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/1640aa97
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/1640aa97
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/1640aa97
Branch: refs/heads/master
Commit: 1640aa9796879405c7112de3065c7fbe88064e8a
Parents: 31c440b
Author: Zoram Thanga <zo...@cloudera.com>
Authored: Tue Oct 17 15:51:26 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Tue Oct 24 21:29:49 2017 +0000
----------------------------------------------------------------------
be/src/exprs/expr-test.cc | 17 +++++++++-----
be/src/runtime/timestamp-value.h | 20 ++++------------
be/src/runtime/timestamp-value.inline.h | 8 +++++++
be/src/service/client-request-state.cc | 8 +++----
be/src/service/client-request-state.h | 8 +++----
be/src/service/impala-http-handler.cc | 31 +++++++++----------------
be/src/service/impala-server.cc | 18 ++++++--------
be/src/service/impala-server.h | 4 ++--
be/src/statestore/statestore-subscriber.cc | 2 +-
be/src/util/common-metrics.cc | 5 ++--
be/src/util/time.h | 5 ++++
11 files changed, 59 insertions(+), 67 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/exprs/expr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index 8fa252b..ed78535 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -51,6 +51,7 @@
#include "runtime/string-value.h"
#include "runtime/timestamp-parse-util.h"
#include "runtime/timestamp-value.h"
+#include "runtime/timestamp-value.inline.h"
#include "service/fe-support.h"
#include "service/impala-server.h"
#include "testutil/impalad-query-executor.h"
@@ -5626,16 +5627,20 @@ TEST_F(ExprTest, TimestampFunctions) {
// Test that the other current time functions are also reasonable.
TimestampValue timestamp_result;
- TimestampValue start_time = TimestampValue::LocalTime();
+ TimestampValue start_time = TimestampValue::FromUnixTimeMicros(UnixMicros());
timestamp_result = ConvertValue<TimestampValue>(GetValue("now()", TYPE_TIMESTAMP));
- EXPECT_BETWEEN(start_time, timestamp_result, TimestampValue::LocalTime());
+ EXPECT_BETWEEN(start_time, timestamp_result,
+ TimestampValue::FromUnixTimeMicros(UnixMicros()));
timestamp_result = ConvertValue<TimestampValue>(GetValue("current_timestamp()",
TYPE_TIMESTAMP));
- EXPECT_BETWEEN(start_time, timestamp_result, TimestampValue::LocalTime());
- const TimestampValue utc_start_time = TimestampValue::UtcTime();
+ EXPECT_BETWEEN(start_time, timestamp_result,
+ TimestampValue::FromUnixTimeMicros(UnixMicros()));
+ const TimestampValue utc_start_time =
+ TimestampValue::UtcFromUnixTimeMicros(UnixMicros());
timestamp_result = ConvertValue<TimestampValue>(GetValue("utc_timestamp()",
TYPE_TIMESTAMP));
- EXPECT_BETWEEN(utc_start_time, timestamp_result, TimestampValue::UtcTime());
+ EXPECT_BETWEEN(utc_start_time, timestamp_result,
+ TimestampValue::UtcFromUnixTimeMicros(UnixMicros()));
// UNIX_TIMESTAMP() has second precision so the comparison start time is shifted back
// a second to ensure an earlier value.
unix_start_time =
@@ -5643,7 +5648,7 @@ TEST_F(ExprTest, TimestampFunctions) {
timestamp_result = ConvertValue<TimestampValue>(GetValue(
"cast(unix_timestamp() as timestamp)", TYPE_TIMESTAMP));
EXPECT_BETWEEN(TimestampValue::FromUnixTime(unix_start_time - 1), timestamp_result,
- TimestampValue::LocalTime());
+ TimestampValue::FromUnixTimeMicros(UnixMicros()));
// Test that UTC and local time represent the same point in time
{
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/runtime/timestamp-value.h
----------------------------------------------------------------------
diff --git a/be/src/runtime/timestamp-value.h b/be/src/runtime/timestamp-value.h
index 0ad7540..556225b 100644
--- a/be/src/runtime/timestamp-value.h
+++ b/be/src/runtime/timestamp-value.h
@@ -106,6 +106,10 @@ class TimestampValue {
return TimestampValue(temp);
}
+ /// Return the corresponding timestamp in local time zone for the Unix time specified in
+ /// microseconds.
+ static TimestampValue FromUnixTimeMicros(int64_t unix_time_micros);
+
/// Return the corresponding timestamp in UTC for the Unix time specified in
/// microseconds.
static TimestampValue UtcFromUnixTimeMicros(int64_t unix_time_micros);
@@ -120,22 +124,6 @@ class TimestampValue {
return TimestampValue(temp);
}
- /// Returns the current local time with microsecond accuracy. This should not be used
- /// to time something because it is affected by adjustments to the system clock such
- /// as a daylight savings or a manual correction by a system admin. For timings, use
- /// functions in util/time.h.
- static TimestampValue LocalTime() {
- return TimestampValue(boost::posix_time::microsec_clock::local_time());
- }
-
- /// Returns the current Coordinated Universal Time ("UTC") with microsecond accuracy.
- /// This should not be used to time something because it is affected by adjustments to
- /// the system clock such as a manual correction by a system admin. For timings,
- /// use functions in util/time.h.
- static TimestampValue UtcTime() {
- return TimestampValue(boost::posix_time::microsec_clock::universal_time());
- }
-
/// Returns a TimestampValue converted from a TimestampVal. The caller must ensure
/// the TimestampVal does not represent a NULL.
static TimestampValue FromTimestampVal(const impala_udf::TimestampVal& udf_value) {
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/runtime/timestamp-value.inline.h
----------------------------------------------------------------------
diff --git a/be/src/runtime/timestamp-value.inline.h b/be/src/runtime/timestamp-value.inline.h
index eff78d0..cdabc20 100644
--- a/be/src/runtime/timestamp-value.inline.h
+++ b/be/src/runtime/timestamp-value.inline.h
@@ -36,6 +36,14 @@ inline TimestampValue TimestampValue::UtcFromUnixTimeMicros(int64_t unix_time_mi
return TimestampValue(temp);
}
+inline TimestampValue TimestampValue::FromUnixTimeMicros(int64_t unix_time_micros) {
+ int64_t ts_seconds = unix_time_micros / MICROS_PER_SEC;
+ int64_t micros_part = unix_time_micros - (ts_seconds * MICROS_PER_SEC);
+ boost::posix_time::ptime temp = UnixTimeToLocalPtime(ts_seconds);
+ temp += boost::posix_time::microseconds(micros_part);
+ return TimestampValue(temp);
+}
+
/// Interpret 'this' as a timestamp in UTC and convert to unix time.
/// Returns false if the conversion failed ('unix_time' will be undefined), otherwise
/// true.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/service/client-request-state.cc
----------------------------------------------------------------------
diff --git a/be/src/service/client-request-state.cc b/be/src/service/client-request-state.cc
index cb542d0..ec4768e 100644
--- a/be/src/service/client-request-state.cc
+++ b/be/src/service/client-request-state.cc
@@ -88,7 +88,7 @@ ClientRequestState::ClientRequestState(
fetched_rows_(false),
frontend_(frontend),
parent_server_(server),
- start_time_(TimestampValue::LocalTime()) {
+ start_time_us_(UnixMicros()) {
#ifndef NDEBUG
profile_->AddInfoString("DEBUG MODE WARNING", "Query profile created while running a "
"DEBUG build of Impala. Use RELEASE builds to measure query performance.");
@@ -106,7 +106,7 @@ ClientRequestState::ClientRequestState(
summary_profile_->AddInfoString("HiveServer2 Protocol Version",
Substitute("V$0", 1 + session->hs2_version));
}
- summary_profile_->AddInfoString("Start Time", start_time().ToString());
+ summary_profile_->AddInfoString("Start Time", ToStringFromUnixMicros(start_time_us()));
summary_profile_->AddInfoString("End Time", "");
summary_profile_->AddInfoString("Query Type", "N/A");
summary_profile_->AddInfoString("Query State", PrintQueryState(query_state_));
@@ -564,8 +564,8 @@ void ClientRequestState::Done() {
}
unique_lock<mutex> l(lock_);
- end_time_ = TimestampValue::LocalTime();
- summary_profile_->AddInfoString("End Time", end_time().ToString());
+ end_time_us_ = UnixMicros();
+ summary_profile_->AddInfoString("End Time", ToStringFromUnixMicros(end_time_us()));
query_events_->MarkEvent("Unregister query");
// Update result set cache metrics, and update mem limit accounting before tearing
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/service/client-request-state.h
----------------------------------------------------------------------
diff --git a/be/src/service/client-request-state.h b/be/src/service/client-request-state.h
index 1c015c3..0ebdda9 100644
--- a/be/src/service/client-request-state.h
+++ b/be/src/service/client-request-state.h
@@ -186,8 +186,8 @@ class ClientRequestState {
}
const RuntimeProfile* profile() const { return profile_; }
const RuntimeProfile* summary_profile() const { return summary_profile_; }
- const TimestampValue& start_time() const { return start_time_; }
- const TimestampValue& end_time() const { return end_time_; }
+ int64_t start_time_us() const { return start_time_us_; }
+ int64_t end_time_us() const { return end_time_us_; }
const std::string& sql_stmt() const { return query_ctx_.client_request.stmt; }
const TQueryOptions& query_options() const {
return query_ctx_.client_request.query_options;
@@ -338,8 +338,8 @@ class ClientRequestState {
/// catalog update request. Not owned.
ImpalaServer* parent_server_;
- /// Start/end time of the query
- TimestampValue start_time_, end_time_;
+ /// Start/end time of the query, in Unix microseconds.
+ int64_t start_time_us_, end_time_us_;
/// Executes a local catalog operation (an operation that does not need to execute
/// against the catalog service). Includes USE, SHOW, DESCRIBE, and EXPLAIN statements.
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/service/impala-http-handler.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-http-handler.cc b/be/src/service/impala-http-handler.cc
index e93aacf..d4ef52f 100644
--- a/be/src/service/impala-http-handler.cc
+++ b/be/src/service/impala-http-handler.cc
@@ -292,21 +292,17 @@ void ImpalaHttpHandler::QueryStateToJson(const ImpalaServer::QueryStateRecord& r
document->GetAllocator());
value->AddMember("stmt_type", stmt_type, document->GetAllocator());
- Value start_time(record.start_time.ToString().c_str(), document->GetAllocator());
+ Value start_time(ToStringFromUnixMicros(record.start_time_us).c_str(),
+ document->GetAllocator());
value->AddMember("start_time", start_time, document->GetAllocator());
- Value end_time(record.end_time.ToString().c_str(), document->GetAllocator());
+ Value end_time(ToStringFromUnixMicros(record.end_time_us).c_str(),
+ document->GetAllocator());
value->AddMember("end_time", end_time, document->GetAllocator());
- const TimestampValue& end_timestamp =
- record.end_time.HasDate() ? record.end_time : TimestampValue::LocalTime();
- double ut_end_time, ut_start_time;
- double duration = 0.0;
- if (LIKELY(end_timestamp.ToSubsecondUnixTime(&ut_end_time))
- && LIKELY(record.start_time.ToSubsecondUnixTime(&ut_start_time))) {
- duration = ut_end_time - ut_start_time;
- }
- const string& printed_duration = PrettyPrinter::Print(duration, TUnit::TIME_S);
+ int64_t duration_us = record.end_time_us - record.start_time_us;
+ const string& printed_duration = PrettyPrinter::Print(duration_us * NANOS_PER_MICRO,
+ TUnit::TIME_NS);
Value val_duration(printed_duration.c_str(), document->GetAllocator());
value->AddMember("duration", val_duration, document->GetAllocator());
@@ -463,19 +459,14 @@ void ImpalaHttpHandler::SessionsHandler(const Webserver::ArgumentMap& args,
Value default_db(state->database.c_str(), document->GetAllocator());
session_json.AddMember("default_database", default_db, document->GetAllocator());
- TimestampValue local_start_time = TimestampValue::FromUnixTime(
- session.second->start_time_ms / 1000);
- local_start_time.UtcToLocal();
- Value start_time(local_start_time.ToString().c_str(), document->GetAllocator());
+ Value start_time(ToStringFromUnixMillis(session.second->start_time_ms,
+ TimePrecision::Second).c_str(), document->GetAllocator());
session_json.AddMember("start_time", start_time, document->GetAllocator());
session_json.AddMember(
"start_time_sort", session.second->start_time_ms, document->GetAllocator());
- TimestampValue local_last_accessed = TimestampValue::FromUnixTime(
- session.second->last_accessed_ms / 1000);
- local_last_accessed.UtcToLocal();
- Value last_accessed(
- local_last_accessed.ToString().c_str(), document->GetAllocator());
+ Value last_accessed(ToStringFromUnixMillis(session.second->last_accessed_ms,
+ TimePrecision::Second).c_str(), document->GetAllocator());
session_json.AddMember("last_accessed", last_accessed, document->GetAllocator());
session_json.AddMember(
"last_accessed_sort", session.second->last_accessed_ms, document->GetAllocator());
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/service/impala-server.cc
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index 50fe25e..bc8613b 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -470,7 +470,7 @@ Status ImpalaServer::LogAuditRecord(const ClientRequestState& request_state,
writer.String("session_id");
writer.String(PrintId(request_state.session_id()).c_str());
writer.String("start_time");
- writer.String(request_state.start_time().ToString().c_str());
+ writer.String(ToStringFromUnixMicros(request_state.start_time_us()).c_str());
writer.String("authorization_failure");
writer.Bool(Frontend::IsAuthorizationError(request_state.query_status()));
writer.String("status");
@@ -1005,12 +1005,8 @@ Status ImpalaServer::UnregisterQuery(const TUniqueId& query_id, bool check_infli
request_state->Done();
- double ut_end_time, ut_start_time;
- double duration_ms = 0.0;
- if (LIKELY(request_state->end_time().ToSubsecondUnixTime(&ut_end_time))
- && LIKELY(request_state->start_time().ToSubsecondUnixTime(&ut_start_time))) {
- duration_ms = 1000 * (ut_end_time - ut_start_time);
- }
+ int64_t duration_us = request_state->end_time_us() - request_state->start_time_us();
+ int64_t duration_ms = duration_us / MICROS_PER_MILLI;
// duration_ms can be negative when the local timezone changes during query execution.
if (duration_ms >= 0) {
@@ -1665,8 +1661,8 @@ ImpalaServer::QueryStateRecord::QueryStateRecord(const ClientRequestState& reque
stmt_type = request.stmt_type;
effective_user = request_state.effective_user();
default_db = request_state.default_db();
- start_time = request_state.start_time();
- end_time = request_state.end_time();
+ start_time_us = request_state.start_time_us();
+ end_time_us = request_state.end_time_us();
has_coord = false;
Coordinator* coord = request_state.coord();
@@ -1711,8 +1707,8 @@ ImpalaServer::QueryStateRecord::QueryStateRecord(const ClientRequestState& reque
bool ImpalaServer::QueryStateRecordLessThan::operator() (
const QueryStateRecord& lhs, const QueryStateRecord& rhs) const {
- if (lhs.start_time == rhs.start_time) return lhs.id < rhs.id;
- return lhs.start_time < rhs.start_time;
+ if (lhs.start_time_us == rhs.start_time_us) return lhs.id < rhs.id;
+ return lhs.start_time_us < rhs.start_time_us;
}
void ImpalaServer::ConnectionStart(
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/service/impala-server.h
----------------------------------------------------------------------
diff --git a/be/src/service/impala-server.h b/be/src/service/impala-server.h
index dbd2877..1653765 100644
--- a/be/src/service/impala-server.h
+++ b/be/src/service/impala-server.h
@@ -619,8 +619,8 @@ class ImpalaServer : public ImpalaServiceIf,
/// The state of the query as of this snapshot
beeswax::QueryState::type query_state;
- /// Start and end time of the query
- TimestampValue start_time, end_time;
+ /// Start and end time of the query, in Unix microseconds.
+ int64_t start_time_us, end_time_us;
/// Summary of execution for this query.
TExecSummary exec_summary;
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/statestore/statestore-subscriber.cc
----------------------------------------------------------------------
diff --git a/be/src/statestore/statestore-subscriber.cc b/be/src/statestore/statestore-subscriber.cc
index fa839aa..4c5d480 100644
--- a/be/src/statestore/statestore-subscriber.cc
+++ b/be/src/statestore/statestore-subscriber.cc
@@ -275,7 +275,7 @@ void StatestoreSubscriber::RecoveryModeChecker() {
// we would otherwise have to cache updates here.
last_recovery_duration_metric_->set_value(
recovery_timer.ElapsedTime() / (1000.0 * 1000.0 * 1000.0));
- last_recovery_time_metric_->set_value(TimestampValue::LocalTime().ToString());
+ last_recovery_time_metric_->set_value(CurrentTimeString());
}
SleepForMs(SLEEP_INTERVAL_MS);
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/util/common-metrics.cc
----------------------------------------------------------------------
diff --git a/be/src/util/common-metrics.cc b/be/src/util/common-metrics.cc
index 94c32b5..d147862 100644
--- a/be/src/util/common-metrics.cc
+++ b/be/src/util/common-metrics.cc
@@ -16,7 +16,7 @@
// under the License.
#include "util/common-metrics.h"
-#include "runtime/timestamp-value.h"
+#include "util/time.h"
#include <kudu/client/client.h>
namespace impala {
@@ -33,8 +33,7 @@ void CommonMetrics::InitCommonMetrics(MetricGroup* metric_group) {
KUDU_CLIENT_VERSION = metric_group->AddProperty<string>(
KUDU_CLIENT_VERSION_METRIC_NAME, kudu::client::GetShortVersionString());
- // TODO: IMPALA-5599: Clean up non-TIMESTAMP usages of TimestampValue
- PROCESS_START_TIME->set_value(TimestampValue::LocalTime().ToString());
+ PROCESS_START_TIME->set_value(CurrentTimeString());
}
}
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1640aa97/be/src/util/time.h
----------------------------------------------------------------------
diff --git a/be/src/util/time.h b/be/src/util/time.h
index 5e9a4fd..d636183 100644
--- a/be/src/util/time.h
+++ b/be/src/util/time.h
@@ -103,5 +103,10 @@ std::string ToStringFromUnixMicros(int64_t us,
std::string ToUtcStringFromUnixMicros(int64_t us,
TimePrecision p = TimePrecision::Microsecond);
+/// Convenience function to convert the current time, derived from UnixMicros(),
+/// to a date-time string in the local time zone.
+inline std::string CurrentTimeString() {
+ return ToStringFromUnixMicros(UnixMicros());
+}
} // namespace impala
#endif