impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tarmstr...@apache.org
Subject [25/32] incubator-impala git commit: Buffer pool: Add basic counters to buffer pool client
Date Tue, 18 Oct 2016 16:43:48 GMT
Buffer pool: Add basic counters to buffer pool client

Change-Id: I9a5a57b7cfccf67ee498e68964f1e077075ee325
Reviewed-on: http://gerrit.cloudera.org:8080/4714
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
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/e3a08914
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/e3a08914
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/e3a08914

Branch: refs/heads/hadoop-next
Commit: e3a08914451a63fe65e8f66afc743739f4570ba4
Parents: 07da767
Author: Tim Armstrong <tarmstrong@cloudera.com>
Authored: Fri Oct 7 09:23:59 2016 -0700
Committer: Internal Jenkins <cloudera-hudson@gerrit.cloudera.org>
Committed: Tue Oct 18 04:48:43 2016 +0000

----------------------------------------------------------------------
 be/src/bufferpool/buffer-pool-counters.h      | 47 ++++++++++++++++++++++
 be/src/bufferpool/buffer-pool-test.cc         | 30 +++++++-------
 be/src/bufferpool/buffer-pool.cc              | 33 ++++++++++++---
 be/src/bufferpool/buffer-pool.h               | 15 +++++--
 be/src/bufferpool/reservation-tracker-test.cc |  9 +----
 5 files changed, 102 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e3a08914/be/src/bufferpool/buffer-pool-counters.h
----------------------------------------------------------------------
diff --git a/be/src/bufferpool/buffer-pool-counters.h b/be/src/bufferpool/buffer-pool-counters.h
new file mode 100644
index 0000000..6f3801e
--- /dev/null
+++ b/be/src/bufferpool/buffer-pool-counters.h
@@ -0,0 +1,47 @@
+// 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.
+
+#ifndef IMPALA_BUFFER_POOL_COUNTERS_H
+#define IMPALA_BUFFER_POOL_COUNTERS_H
+
+#include "util/runtime-profile.h"
+
+namespace impala {
+
+/// A set of counters for each buffer pool client.
+struct BufferPoolClientCounters {
+ public:
+  /// Amount of time spent trying to get a buffer.
+  RuntimeProfile::Counter* get_buffer_time;
+
+  /// Amount of time spent waiting for reads from disk to complete.
+  RuntimeProfile::Counter* read_wait_time;
+
+  /// Amount of time spent waiting for writes to disk to complete.
+  RuntimeProfile::Counter* write_wait_time;
+
+  /// The peak total size of unpinned buffers.
+  RuntimeProfile::HighWaterMarkCounter* peak_unpinned_bytes;
+
+  /// The total bytes of data unpinned. Every time a page's pin count goes from 1 to 0,
+  /// this counter is incremented by the page size.
+  RuntimeProfile::Counter* total_unpinned_bytes;
+};
+
+}
+
+#endif

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e3a08914/be/src/bufferpool/buffer-pool-test.cc
----------------------------------------------------------------------
diff --git a/be/src/bufferpool/buffer-pool-test.cc b/be/src/bufferpool/buffer-pool-test.cc
index 16cf12c..793bcb9 100644
--- a/be/src/bufferpool/buffer-pool-test.cc
+++ b/be/src/bufferpool/buffer-pool-test.cc
@@ -15,7 +15,6 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include <gtest/gtest.h>
 #include <boost/bind.hpp>
 #include <boost/scoped_ptr.hpp>
 #include <boost/thread/thread.hpp>
@@ -29,7 +28,7 @@
 #include "common/init.h"
 #include "common/object-pool.h"
 #include "testutil/death-test-util.h"
-#include "testutil/test-macros.h"
+#include "testutil/gtest-util.h"
 
 #include "common/names.h"
 
@@ -125,7 +124,8 @@ void BufferPoolTest::RegisterQueriesAndClients(BufferPool* pool, int query_id_hi
       EXPECT_TRUE(
           client_reservations[i][j].IncreaseReservationToFit(initial_client_reservation));
       string name = Substitute("Client $0 for query $1", j, query_id);
-      EXPECT_OK(pool->RegisterClient(name, &client_reservations[i][j], &clients[i][j]));
+      EXPECT_OK(pool->RegisterClient(
+          name, &client_reservations[i][j], NewProfile(), &clients[i][j]));
     }
 
     for (int j = 0; j < clients_per_query; ++j) {
@@ -209,7 +209,7 @@ TEST_F(BufferPoolTest, PageCreation) {
   client_tracker->InitChildTracker(NewProfile(), &global_reservations_, NULL, total_mem);
   ASSERT_TRUE(client_tracker->IncreaseReservation(total_mem));
   BufferPool::Client client;
-  ASSERT_OK(pool.RegisterClient("test client", client_tracker, &client));
+  ASSERT_OK(pool.RegisterClient("test client", client_tracker, NewProfile(), &client));
 
   vector<BufferPool::PageHandle> handles(num_pages);
 
@@ -256,7 +256,7 @@ TEST_F(BufferPoolTest, BufferAllocation) {
   client_tracker->InitChildTracker(NewProfile(), &global_reservations_, NULL, total_mem);
   ASSERT_TRUE(client_tracker->IncreaseReservationToFit(total_mem));
   BufferPool::Client client;
-  ASSERT_OK(pool.RegisterClient("test client", client_tracker, &client));
+  ASSERT_OK(pool.RegisterClient("test client", client_tracker, NewProfile(), &client));
 
   vector<BufferPool::BufferHandle> handles(num_buffers);
 
@@ -302,7 +302,8 @@ TEST_F(BufferPoolTest, BufferTransfer) {
     client_trackers[i].InitChildTracker(
         NewProfile(), &global_reservations_, NULL, TEST_BUFFER_LEN);
     ASSERT_TRUE(client_trackers[i].IncreaseReservationToFit(TEST_BUFFER_LEN));
-    ASSERT_OK(pool.RegisterClient("test client", &client_trackers[i], &clients[i]));
+    ASSERT_OK(pool.RegisterClient(
+        "test client", &client_trackers[i], NewProfile(), &clients[i]));
   }
 
   // Transfer the page around between the clients repeatedly in a circle.
@@ -344,7 +345,7 @@ TEST_F(BufferPoolTest, Pin) {
       NewProfile(), &global_reservations_, NULL, child_reservation);
   ASSERT_TRUE(client_tracker->IncreaseReservationToFit(child_reservation));
   BufferPool::Client client;
-  ASSERT_OK(pool.RegisterClient("test client", client_tracker, &client));
+  ASSERT_OK(pool.RegisterClient("test client", client_tracker, NewProfile(), &client));
 
   BufferPool::PageHandle handle1, handle2;
 
@@ -395,7 +396,7 @@ TEST_F(BufferPoolTest, PinWithoutReservation) {
   client_tracker->InitChildTracker(
       NewProfile(), &global_reservations_, NULL, TEST_BUFFER_LEN);
   BufferPool::Client client;
-  ASSERT_OK(pool.RegisterClient("test client", client_tracker, &client));
+  ASSERT_OK(pool.RegisterClient("test client", client_tracker, NewProfile(), &client));
 
   BufferPool::PageHandle handle;
   IMPALA_ASSERT_DEBUG_DEATH(pool.CreatePage(&client, TEST_BUFFER_LEN, &handle), "");
@@ -423,7 +424,7 @@ TEST_F(BufferPoolTest, ExtractBuffer) {
       NewProfile(), &global_reservations_, NULL, child_reservation);
   ASSERT_TRUE(client_tracker->IncreaseReservationToFit(child_reservation));
   BufferPool::Client client;
-  ASSERT_OK(pool.RegisterClient("test client", client_tracker, &client));
+  ASSERT_OK(pool.RegisterClient("test client", client_tracker, NewProfile(), &client));
 
   BufferPool::PageHandle page;
   BufferPool::BufferHandle buffer;
@@ -499,7 +500,7 @@ void BufferPoolTest::CreatePageLoop(
   ReservationTracker client_tracker;
   client_tracker.InitChildTracker(NewProfile(), parent_tracker, NULL, TEST_BUFFER_LEN);
   BufferPool::Client client;
-  ASSERT_OK(pool->RegisterClient("test client", &client_tracker, &client));
+  ASSERT_OK(pool->RegisterClient("test client", &client_tracker, NewProfile(), &client));
   for (int i = 0; i < num_ops; ++i) {
     BufferPool::PageHandle handle;
     ASSERT_TRUE(client_tracker.IncreaseReservation(TEST_BUFFER_LEN));
@@ -525,7 +526,8 @@ TEST_F(BufferPoolTest, CapacityExhausted) {
   BufferPool::PageHandle handle1, handle2, handle3;
 
   BufferPool::Client client;
-  ASSERT_OK(pool.RegisterClient("test client", &global_reservations_, &client));
+  ASSERT_OK(
+      pool.RegisterClient("test client", &global_reservations_, NewProfile(), &client));
   ASSERT_TRUE(global_reservations_.IncreaseReservation(TEST_BUFFER_LEN));
   ASSERT_OK(pool.CreatePage(&client, TEST_BUFFER_LEN, &handle1));
 
@@ -549,8 +551,4 @@ TEST_F(BufferPoolTest, CapacityExhausted) {
 }
 }
 
-int main(int argc, char** argv) {
-  ::testing::InitGoogleTest(&argc, argv);
-  impala::InitCommonRuntime(argc, argv, true, impala::TestInfo::BE_TEST);
-  return RUN_ALL_TESTS();
-}
+IMPALA_TEST_MAIN();

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e3a08914/be/src/bufferpool/buffer-pool.cc
----------------------------------------------------------------------
diff --git a/be/src/bufferpool/buffer-pool.cc b/be/src/bufferpool/buffer-pool.cc
index eaa4262..3035694 100644
--- a/be/src/bufferpool/buffer-pool.cc
+++ b/be/src/bufferpool/buffer-pool.cc
@@ -25,6 +25,7 @@
 #include "common/names.h"
 #include "gutil/strings/substitute.h"
 #include "util/bit-util.h"
+#include "util/runtime-profile-counters.h"
 #include "util/uid-util.h"
 
 namespace impala {
@@ -176,14 +177,26 @@ BufferPool::~BufferPool() {
 }
 
 Status BufferPool::RegisterClient(
-    const string& name, ReservationTracker* reservation, Client* client) {
+    const string& name, ReservationTracker* reservation, RuntimeProfile* profile,
+    Client* client) {
   DCHECK(!client->is_registered());
   DCHECK(reservation != NULL);
+  client->InitCounters(profile);
   client->reservation_ = reservation;
   client->name_ = name;
   return Status::OK();
 }
 
+void BufferPool::Client::InitCounters(RuntimeProfile* profile) {
+  counters_.get_buffer_time = ADD_TIMER(profile, "BufferPoolGetBufferTime");
+  counters_.read_wait_time = ADD_TIMER(profile, "BufferPoolReadWaitTime");
+  counters_.write_wait_time = ADD_TIMER(profile, "BufferPoolWriteWaitTime");
+  counters_.peak_unpinned_bytes =
+      profile->AddHighWaterMarkCounter("BufferPoolPeakUnpinnedBytes", TUnit::BYTES);
+  counters_.total_unpinned_bytes =
+      ADD_COUNTER(profile, "BufferPoolTotalUnpinnedBytes", TUnit::BYTES);
+}
+
 void BufferPool::DeregisterClient(Client* client) {
   if (!client->is_registered()) return;
   client->reservation_->Close();
@@ -256,13 +269,16 @@ Status BufferPool::Pin(Client* client, PageHandle* handle) {
   Page* page = handle->page_;
   {
     lock_guard<SpinLock> pl(page->lock); // Lock page while we work on its state.
-    if (!page->buffer.is_open()) {
-      // No changes have been made to state yet, so we can cleanly return on error.
-      RETURN_IF_ERROR(AllocateBufferInternal(client, page->len, &page->buffer));
+    if (page->pin_count == 0)  {
+      if (!page->buffer.is_open()) {
+        // No changes have been made to state yet, so we can cleanly return on error.
+        RETURN_IF_ERROR(AllocateBufferInternal(client, page->len, &page->buffer));
+
+        // TODO: will need to initiate/wait for read if the page is not in-memory.
+      }
+      COUNTER_ADD(client->counters_.peak_unpinned_bytes, -handle->len());
     }
     page->IncrementPinCount(handle);
-
-    // TODO: will need to initiate/wait for read if the page is not in-memory.
   }
 
   client->reservation_->AllocateFrom(page->len);
@@ -286,12 +302,16 @@ void BufferPool::UnpinLocked(Client* client, PageHandle* handle) {
   page->DecrementPinCount(handle);
   client->reservation_->ReleaseTo(page->len);
 
+  COUNTER_ADD(client->counters_.total_unpinned_bytes, handle->len());
+  COUNTER_ADD(client->counters_.peak_unpinned_bytes, handle->len());
+
   // TODO: can evict now. Only need to preserve contents if 'page->dirty' is true.
 }
 
 void BufferPool::ExtractBuffer(
     Client* client, PageHandle* page_handle, BufferHandle* buffer_handle) {
   DCHECK(page_handle->is_pinned());
+
   DCHECK_EQ(page_handle->client_, client);
 
   Page* page = page_handle->page_;
@@ -316,6 +336,7 @@ Status BufferPool::AllocateBufferInternal(
   DCHECK(!buffer->is_open());
   DCHECK_GE(len, min_buffer_len_);
   DCHECK_EQ(len, BitUtil::RoundUpToPowerOfTwo(len));
+  SCOPED_TIMER(client->counters_.get_buffer_time);
 
   // If there is headroom in 'buffer_bytes_remaining_', we can just allocate a new buffer.
   if (TryDecreaseBufferBytesRemaining(len)) {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e3a08914/be/src/bufferpool/buffer-pool.h
----------------------------------------------------------------------
diff --git a/be/src/bufferpool/buffer-pool.h b/be/src/bufferpool/buffer-pool.h
index 44b5574..6a9641d 100644
--- a/be/src/bufferpool/buffer-pool.h
+++ b/be/src/bufferpool/buffer-pool.h
@@ -24,6 +24,7 @@
 #include <string>
 
 #include "bufferpool/buffer-allocator.h"
+#include "bufferpool/buffer-pool-counters.h"
 #include "common/atomic.h"
 #include "common/status.h"
 #include "gutil/macros.h"
@@ -167,10 +168,11 @@ class BufferPool {
 
   /// Register a client. Returns an error status and does not register the client if the
   /// arguments are invalid. 'name' is an arbitrary name used to identify the client in
-  /// any errors messages or logging. 'client' is the client to register. 'client' should
-  /// not already be registered.
+  /// any errors messages or logging. Counters for this client are added to the (non-NULL)
+  /// 'profile'. 'client' is the client to register. 'client' should not already be
+  /// registered.
   Status RegisterClient(const std::string& name, ReservationTracker* reservation,
-      Client* client);
+      RuntimeProfile* profile, Client* client);
 
   /// Deregister 'client' if it is registered. Idempotent.
   void DeregisterClient(Client* client);
@@ -305,12 +307,19 @@ class BufferPool::Client {
   friend class BufferPool;
   DISALLOW_COPY_AND_ASSIGN(Client);
 
+  /// Initialize 'counters_' and add the counters to 'profile'.
+  void InitCounters(RuntimeProfile* profile);
+
   /// A name identifying the client.
   std::string name_;
 
   /// The reservation tracker for the client. NULL means the client isn't registered.
   /// All pages pinned by the client count as usage against 'reservation_'.
   ReservationTracker* reservation_;
+
+  /// The RuntimeProfile counters for this client. All non-NULL if is_registered()
+  /// is true.
+  BufferPoolClientCounters counters_;
 };
 
 /// A handle to a buffer allocated from the buffer pool. Each BufferHandle should only

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e3a08914/be/src/bufferpool/reservation-tracker-test.cc
----------------------------------------------------------------------
diff --git a/be/src/bufferpool/reservation-tracker-test.cc b/be/src/bufferpool/reservation-tracker-test.cc
index 93bf7b8..66ce287 100644
--- a/be/src/bufferpool/reservation-tracker-test.cc
+++ b/be/src/bufferpool/reservation-tracker-test.cc
@@ -15,7 +15,6 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include <gtest/gtest.h>
 #include <limits>
 #include <string>
 #include <vector>
@@ -24,7 +23,7 @@
 #include "common/init.h"
 #include "common/object-pool.h"
 #include "runtime/mem-tracker.h"
-#include "testutil/test-macros.h"
+#include "testutil/gtest-util.h"
 
 #include "common/names.h"
 
@@ -376,8 +375,4 @@ TEST_F(ReservationTrackerTest, MemTrackerIntegrationMultiLevel) {
 }
 }
 
-int main(int argc, char** argv) {
-  ::testing::InitGoogleTest(&argc, argv);
-  impala::InitCommonRuntime(argc, argv, true, impala::TestInfo::BE_TEST);
-  return RUN_ALL_TESTS();
-}
+IMPALA_TEST_MAIN();


Mime
View raw message