impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mar...@apache.org
Subject incubator-impala git commit: IMPALA-3988: Only use first 96 bits of query id
Date Mon, 22 Aug 2016 17:16:58 GMT
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 <marcel@cloudera.com>
Tested-by: Marcel Kornacker <marcel@cloudera.com>


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 <marcel@cloudera.com>
Authored: Fri Aug 19 09:32:22 2016 -0700
Committer: Marcel Kornacker <marcel@cloudera.com>
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


Mime
View raw message