Repository: incubator-impala Updated Branches: refs/heads/master b251fb061 -> b69e469e9
IMPALA-3988: Only use first 96 bits of query id This adds utility functions in uid-util.h to create query and instance ids and convert between them. It also adapts SimpleScheduler to utilize those functions when creating the instance id (TPlanFragmentInstanceCtx.fragment_instance_id). Change-Id: Ia56a03ed9a1d7e77c72b66a01cd48c5b6bf3624f Reviewed-on: http://gerrit.cloudera.org:8080/4065 Reviewed-by: Marcel Kornacker <[email protected]> Tested-by: Marcel Kornacker <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/b69e469e Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/b69e469e Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/b69e469e Branch: refs/heads/master Commit: b69e469e99e4b0c08bc4718eb55b041cf7c11ffe Parents: b251fb0 Author: Marcel Kornacker <[email protected]> Authored: Fri Aug 19 09:32:22 2016 -0700 Committer: Marcel Kornacker <[email protected]> Committed: Mon Aug 22 17:15:37 2016 +0000 ---------------------------------------------------------------------- be/src/scheduling/simple-scheduler.cc | 11 ++---- be/src/service/impala-server.cc | 2 +- be/src/util/CMakeLists.txt | 1 + be/src/util/uid-util-test.cc | 49 +++++++++++++++++++++++++ be/src/util/uid-util.h | 34 +++++++++++++++++ common/thrift/ImpalaInternalService.thrift | 9 ++++- 6 files changed, 96 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b69e469e/be/src/scheduling/simple-scheduler.cc ---------------------------------------------------------------------- diff --git a/be/src/scheduling/simple-scheduler.cc b/be/src/scheduling/simple-scheduler.cc index a4d866f..5c05557 100644 --- a/be/src/scheduling/simple-scheduler.cc +++ b/be/src/scheduling/simple-scheduler.cc @@ -502,14 +502,9 @@ void SimpleScheduler::ComputeFragmentExecParams(const TQueryExecRequest& exec_re int64_t num_fragment_instances = 0; for (FragmentExecParams& params: *fragment_exec_params) { for (int j = 0; j < params.hosts.size(); ++j) { - int instance_num = num_fragment_instances + j; - // we add instance_num to query_id.lo to create a globally-unique instance id - TUniqueId instance_id; - instance_id.hi = schedule->query_id().hi; - DCHECK_LT( - schedule->query_id().lo, numeric_limits<int64_t>::max() - instance_num - 1); - instance_id.lo = schedule->query_id().lo + instance_num + 1; - params.instance_ids.push_back(instance_id); + int instance_idx = num_fragment_instances + j; + params.instance_ids.push_back( + CreateInstanceId(schedule->query_id(), instance_idx)); } num_fragment_instances += params.hosts.size(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b69e469e/be/src/service/impala-server.cc ---------------------------------------------------------------------- diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc index 9bb8c77..c749a6a 100644 --- a/be/src/service/impala-server.cc +++ b/be/src/service/impala-server.cc @@ -856,7 +856,7 @@ void ImpalaServer::PrepareQueryContext(TQueryCtx* query_ctx) { // thread-safe). random_generator uuid_generator; uuid query_uuid = uuid_generator(); - UUIDToTUniqueId(query_uuid, &query_ctx->query_id); + query_ctx->query_id = UuidToQueryId(query_uuid); } Status ImpalaServer::RegisterQuery(shared_ptr<SessionState> session_state, http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b69e469e/be/src/util/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/be/src/util/CMakeLists.txt b/be/src/util/CMakeLists.txt index 3421c46..dc2df01 100644 --- a/be/src/util/CMakeLists.txt +++ b/be/src/util/CMakeLists.txt @@ -141,3 +141,4 @@ ADD_BE_TEST(fixed-size-hash-table-test) ADD_BE_TEST(bloom-filter-test) ADD_BE_TEST(logging-support-test) ADD_BE_TEST(hdfs-util-test) +ADD_BE_TEST(uid-util-test) http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b69e469e/be/src/util/uid-util-test.cc ---------------------------------------------------------------------- diff --git a/be/src/util/uid-util-test.cc b/be/src/util/uid-util-test.cc new file mode 100644 index 0000000..a789562 --- /dev/null +++ b/be/src/util/uid-util-test.cc @@ -0,0 +1,49 @@ +// 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. + +#include <stdlib.h> +#include <stdio.h> +#include <iostream> +#include <gtest/gtest.h> + +#include "util/uid-util.h" + +namespace impala { + +TEST(UidUtil, FragmentInstanceId) { + boost::uuids::random_generator uuid_generator; + boost::uuids::uuid query_uuid = uuid_generator(); + TUniqueId query_id = UuidToQueryId(query_uuid); + + for (int i = 0; i < 100; ++i) { + TUniqueId instance_id = CreateInstanceId(query_id, i); + EXPECT_EQ(instance_id.hi, query_id.hi); + + TUniqueId qid = GetQueryId(instance_id); + EXPECT_EQ(qid.hi, query_id.hi); + EXPECT_EQ(qid.lo, query_id.lo); + + EXPECT_EQ(GetInstanceIdx(instance_id), i); + } +} + +} + +int main(int argc, char **argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b69e469e/be/src/util/uid-util.h ---------------------------------------------------------------------- diff --git a/be/src/util/uid-util.h b/be/src/util/uid-util.h index 84684d4..c78a9ea 100644 --- a/be/src/util/uid-util.h +++ b/be/src/util/uid-util.h @@ -19,6 +19,7 @@ #ifndef IMPALA_UTIL_UID_UTIL_H #define IMPALA_UTIL_UID_UTIL_H +#include <boost/functional/hash.hpp> #include <boost/uuid/uuid.hpp> #include <boost/uuid/uuid_generators.hpp> @@ -27,6 +28,7 @@ #include "common/names.h" namespace impala { + /// This function must be called 'hash_value' to be picked up by boost. inline std::size_t hash_value(const impala::TUniqueId& id) { std::size_t seed = 0; @@ -43,6 +45,38 @@ inline void UUIDToTUniqueId(const boost::uuids::uuid& uuid, T* unique_id) { memcpy(&(unique_id->lo), &uuid.data[8], 8); } +/// Query id: uuid with bottom 4 bytes set to 0 +/// Fragment instance id: query id with instance index stored in the bottom 4 bytes + +const int64_t FRAGMENT_IDX_MASK = (1L << 32) - 1; + +inline TUniqueId UuidToQueryId(const boost::uuids::uuid& uuid) { + TUniqueId result; + memcpy(&result.hi, &uuid.data[0], 8); + memcpy(&result.lo, &uuid.data[8], 8); + result.lo &= ~FRAGMENT_IDX_MASK; // zero out bottom 4 bytes + return result; +} + +inline TUniqueId GetQueryId(const TUniqueId& fragment_instance_id) { + TUniqueId result = fragment_instance_id; + result.lo &= ~FRAGMENT_IDX_MASK; // zero out bottom 4 bytes + return result; +} + +inline int32_t GetInstanceIdx(const TUniqueId& fragment_instance_id) { + return fragment_instance_id.lo & FRAGMENT_IDX_MASK; +} + +inline TUniqueId CreateInstanceId( + const TUniqueId& query_id, int32_t instance_idx) { + DCHECK_EQ(GetInstanceIdx(query_id), 0); // well-formed query id + DCHECK_GE(instance_idx, 0); + TUniqueId result = query_id; + result.lo += instance_idx; + return result; +} + template <typename F, typename T> inline T CastTUniqueId(const F& from) { T to; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b69e469e/common/thrift/ImpalaInternalService.thrift ---------------------------------------------------------------------- diff --git a/common/thrift/ImpalaInternalService.thrift b/common/thrift/ImpalaInternalService.thrift index ae3ae27..f4b070f 100644 --- a/common/thrift/ImpalaInternalService.thrift +++ b/common/thrift/ImpalaInternalService.thrift @@ -254,6 +254,7 @@ struct TQueryCtx { 1: required TClientRequest request // A globally unique id assigned to the entire query in the BE. + // The bottom 4 bytes are 0 (for details see be/src/util/uid-util.h). 2: required Types.TUniqueId query_id // Session state including user. @@ -326,7 +327,11 @@ struct TPlanFragmentDestination { // of fragment instances, the query context, the coordinator address, etc. // TODO: for range partitioning, we also need to specify the range boundaries struct TPlanFragmentInstanceCtx { - // the globally unique fragment instance id + // The globally unique fragment instance id. + // Format: query id + query-wide fragment index + // The query-wide fragment index starts at 0, so that the query id + // and the id of the first fragment instance (the coordinator instance) + // are identical. 1: required Types.TUniqueId fragment_instance_id // Index of this fragment instance accross all instances of its parent fragment, @@ -334,6 +339,8 @@ struct TPlanFragmentInstanceCtx { 2: required i32 fragment_instance_idx // Index of this fragment instance in Coordinator::fragment_instance_states_. + // TODO: remove; this is subsumed by the query-wide instance idx embedded + // in the fragment_instance_id 3: required i32 instance_state_idx // Initial scan ranges for each scan node in TPlanFragment.plan_tree
