You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2019/03/07 04:46:08 UTC
[kudu] branch master updated (3e2fcf2 -> 0a05f61)
This is an automated email from the ASF dual-hosted git repository.
adar pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git.
from 3e2fcf2 [docs] Fix table name in Impala docs
new feebbb2 KUDU-2721: support ranges in CPU lists
new 0a05f61 tools: fix output on 'kudu table scan' to add proper locking
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/kudu/gutil/CMakeLists.txt | 1 +
src/kudu/gutil/sysinfo-test.cc | 71 +++++++++++++++++++++++++++++++++++++++++
src/kudu/gutil/sysinfo.cc | 69 +++++++++++++++++++++++++++++++--------
src/kudu/gutil/sysinfo.h | 12 +++++++
src/kudu/tools/table_scanner.cc | 8 +++--
src/kudu/tools/table_scanner.h | 5 ++-
6 files changed, 150 insertions(+), 16 deletions(-)
create mode 100644 src/kudu/gutil/sysinfo-test.cc
[kudu] 02/02: tools: fix output on 'kudu table scan' to add proper
locking
Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 0a05f613e44885ad1be2759ad4944233e5e6df74
Author: Todd Lipcon <to...@apache.org>
AuthorDate: Tue Feb 26 16:28:07 2019 -0800
tools: fix output on 'kudu table scan' to add proper locking
The 'kudu table scan' tool is multi-threaded, but no lock was protecting
output. This would cause output to occasionally interleave like:
(int64 l_orderkey=380530182)
(int64 l_orderkey=352994055)(int64 l_orderkey=380680647)
(int64 l_orderkey=353120262)
(int64 l_orderkey=353314147)
(i.e. two rows would print on the same line, and an extra newline would
be inserted later).
This adds a mutex to protect the output.
Change-Id: I9d7f936d9beaf7eedba46477bf7da2ebdb800eb1
Reviewed-on: http://gerrit.cloudera.org:8080/12617
Reviewed-by: Yingchun Lai <40...@qq.com>
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Kudu Jenkins
---
src/kudu/tools/table_scanner.cc | 8 ++++++--
src/kudu/tools/table_scanner.h | 5 ++++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/kudu/tools/table_scanner.cc b/src/kudu/tools/table_scanner.cc
index a2f2e7b..c8c5525 100644
--- a/src/kudu/tools/table_scanner.cc
+++ b/src/kudu/tools/table_scanner.cc
@@ -303,6 +303,7 @@ void TableScanner::ScannerTask(const vector<KuduScanToken *>& tokens) {
count += batch.NumRows();
total_count_.IncrementBy(batch.NumRows());
if (FLAGS_show_value) {
+ MutexLock l(output_lock_);
for (const auto& row : batch) {
cout << row.ToString() << endl;
}
@@ -311,8 +312,11 @@ void TableScanner::ScannerTask(const vector<KuduScanToken *>& tokens) {
delete scanner;
sw.stop();
- cout << "T " << token->tablet().id() << " scanned count " << count
- << " cost " << sw.elapsed().wall_seconds() << " seconds" << endl;
+ {
+ MutexLock l(output_lock_);
+ cout << "T " << token->tablet().id() << " scanned count " << count
+ << " cost " << sw.elapsed().wall_seconds() << " seconds" << endl;
+ }
}
}
diff --git a/src/kudu/tools/table_scanner.h b/src/kudu/tools/table_scanner.h
index c9589c5..2165390 100644
--- a/src/kudu/tools/table_scanner.h
+++ b/src/kudu/tools/table_scanner.h
@@ -26,6 +26,7 @@
#include "kudu/client/shared_ptr.h"
#include "kudu/gutil/gscoped_ptr.h"
#include "kudu/util/atomic.h"
+#include "kudu/util/mutex.h"
#include "kudu/util/status.h"
#include "kudu/util/threadpool.h"
@@ -57,11 +58,13 @@ private:
void ScannerTask(const vector<KuduScanToken *>& tokens);
void MonitorTask();
-private:
AtomicInt<uint64_t> total_count_;
client::sp::shared_ptr<KuduClient> client_;
std::string table_name_;
gscoped_ptr<ThreadPool> thread_pool_;
+
+ // Protects output to stdout so that rows don't get interleaved.
+ Mutex output_lock_;
};
} // namespace tools
} // namespace kudu
[kudu] 01/02: KUDU-2721: support ranges in CPU lists
Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit feebbb2649a659bbd6b2fdcfc6a347eafbccba2b
Author: Tim Armstrong <ta...@cloudera.com>
AuthorDate: Mon Mar 4 16:05:43 2019 -0800
KUDU-2721: support ranges in CPU lists
Generalise the logic to handle comma-separated ranges.
Testing:
Added unit test for parsing logic to test the various possible
permutations of CPU ranges.
Change-Id: I97311bfbcf70bea069e941b6e7f4f015fb781b3f
Reviewed-on: http://gerrit.cloudera.org:8080/12657
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
src/kudu/gutil/CMakeLists.txt | 1 +
src/kudu/gutil/sysinfo-test.cc | 71 ++++++++++++++++++++++++++++++++++++++++++
src/kudu/gutil/sysinfo.cc | 69 ++++++++++++++++++++++++++++++++--------
src/kudu/gutil/sysinfo.h | 12 +++++++
4 files changed, 140 insertions(+), 13 deletions(-)
diff --git a/src/kudu/gutil/CMakeLists.txt b/src/kudu/gutil/CMakeLists.txt
index 3d487eb..5569650 100644
--- a/src/kudu/gutil/CMakeLists.txt
+++ b/src/kudu/gutil/CMakeLists.txt
@@ -70,3 +70,4 @@ ADD_EXPORTABLE_LIBRARY(gutil
add_kudu_test(strings/string_util-test)
add_kudu_test(strings/numbers-test)
+add_kudu_test(sysinfo-test)
diff --git a/src/kudu/gutil/sysinfo-test.cc b/src/kudu/gutil/sysinfo-test.cc
new file mode 100644
index 0000000..a485bb2
--- /dev/null
+++ b/src/kudu/gutil/sysinfo-test.cc
@@ -0,0 +1,71 @@
+// 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 "kudu/gutil/sysinfo.h"
+
+#include <gtest/gtest.h>
+
+namespace kudu {
+
+TEST(SysInfoTest, ReadMaxCpuIndexTest) {
+ using base::ParseMaxCpuIndex;
+ EXPECT_EQ(0, ParseMaxCpuIndex("0\n"));
+ EXPECT_EQ(1, ParseMaxCpuIndex("1\n"));
+ EXPECT_EQ(7, ParseMaxCpuIndex("0-7\n"));
+ EXPECT_EQ(40, ParseMaxCpuIndex("0-7,30-40\n"));
+ EXPECT_EQ(143, ParseMaxCpuIndex("2,4-127,128-143\n"));
+ EXPECT_EQ(44, ParseMaxCpuIndex("44-44\n"));
+
+ // Don't assume that ranges are in ascending order or non-overlapping,
+ // just in case.
+ EXPECT_EQ(8, ParseMaxCpuIndex("0-7,5-8\n"));
+ EXPECT_EQ(7, ParseMaxCpuIndex("0-7,5-6\n"));
+ EXPECT_EQ(3, ParseMaxCpuIndex("2-3,0-1\n"));
+ EXPECT_EQ(3, ParseMaxCpuIndex("2-3,0\n"));
+ EXPECT_EQ(3, ParseMaxCpuIndex("3,0-2\n"));
+
+ // Invalid inputs.
+ EXPECT_EQ(-1, ParseMaxCpuIndex(""));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex(" "));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("a\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("0\n1\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("\n1\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("\n1"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("0-\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("-2\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("1-9qwerty\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("1-9,0-\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("1,2,3-\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("1,2,a-4\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("1,2,3@4\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("1,2,\n"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("3-2\n"));
+
+ // Overflows in various positions.
+ EXPECT_EQ(-1, ParseMaxCpuIndex("2147483648")); // 2^31
+ EXPECT_EQ(-1, ParseMaxCpuIndex("18446744073709551617")); // 2^64 + 1
+ EXPECT_EQ(-1, ParseMaxCpuIndex("999999999999999999999999999999999999999999999999"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("0-2147483648")); // 2^31
+ EXPECT_EQ(-1, ParseMaxCpuIndex("0-18446744073709551617")); // 2^64 + 1
+ EXPECT_EQ(-1, ParseMaxCpuIndex("0-999999999999999999999999999999999999999999999999"));
+ EXPECT_EQ(-1, ParseMaxCpuIndex("2147483648-1")); // 2^31
+ EXPECT_EQ(-1, ParseMaxCpuIndex("18446744073709551617-1")); // 2^64 + 1
+ EXPECT_EQ(-1, ParseMaxCpuIndex("999999999999999999999999999999999999999999999999-1"));
+}
+
+} // namespace kudu
diff --git a/src/kudu/gutil/sysinfo.cc b/src/kudu/gutil/sysinfo.cc
index 289a4b1..dbdb207 100644
--- a/src/kudu/gutil/sysinfo.cc
+++ b/src/kudu/gutil/sysinfo.cc
@@ -32,7 +32,7 @@
# define PLATFORM_WINDOWS 1
#endif
-
+#include <ctype.h>
#include <fcntl.h> // for open()
#include <unistd.h> // for read()
@@ -51,11 +51,13 @@
#include "kudu/gutil/sysinfo.h"
+#include <algorithm>
#include <cerrno> // for errno
#include <cstdio> // for snprintf(), sscanf()
#include <cstdlib> // for getenv()
#include <cstring> // for memmove(), memchr(), etc.
#include <ctime>
+#include <limits>
#include <ostream>
#include <glog/logging.h>
@@ -66,6 +68,8 @@
#include "kudu/gutil/port.h"
#include "kudu/gutil/walltime.h"
+using std::numeric_limits;
+
// This isn't in the 'base' namespace in tcmallc. But, tcmalloc
// exports these functions, so we need to namespace them to avoid
// the conflict.
@@ -161,21 +165,60 @@ static bool ReadIntFromFile(const char *file, int *value) {
static int ReadMaxCPUIndex() {
char buf[1024];
+ // TODO(tarmstrong): KUDU-2730: 'present' doesn't include CPUs that could be hotplugged
+ // in the future. 'possible' does, but using it instead could result in a blow-up in the
+ // number of per-CPU data structures.
CHECK(SlurpSmallTextFile("/sys/devices/system/cpu/present", buf, arraysize(buf)));
+ int max_idx = ParseMaxCpuIndex(buf);
+ CHECK_GE(max_idx, 0) << "unable to parse max CPU index from: " << buf;
+ return max_idx;
+}
- // On a single-core machine, 'buf' will contain the string '0' with a newline.
- if (strcmp(buf, "0\n") == 0) {
- return 0;
- }
-
- // On multi-core, it will have a CPU range like '0-7'.
- CHECK_EQ(0, memcmp(buf, "0-", 2)) << "bad list of possible CPUs: " << buf;
+int ParseMaxCpuIndex(const char* str) {
+ DCHECK(str != nullptr);
+ const char* pos = str;
+ // Initialize max_idx to invalid so we can just return if we find zero ranges.
+ int max_idx = -1;
+
+ while (true) {
+ const char* range_start = pos;
+ const char* dash = nullptr;
+ // Scan forward until we find the separator indicating end of range, which is always a
+ // newline or comma if the input is valid.
+ for (; *pos != ',' && *pos != '\n'; pos++) {
+ // Check for early end of string - bail here to avoid advancing past end.
+ if (*pos == '\0') return -1;
+ if (*pos == '-') {
+ // Multiple dashes in range is invalid.
+ if (dash != nullptr) return -1;
+ dash = pos;
+ } else if (!isdigit(*pos)) {
+ return -1;
+ }
+ }
- char* max_cpu_str = &buf[2];
- char* err;
- int val = strtol(max_cpu_str, &err, 10);
- CHECK(*err == '\n' || *err == '\0') << "unable to parse max CPU index from: " << buf;
- return val;
+ // At this point we found a range [range_start, pos) comprised of digits and an
+ // optional dash.
+ const char* num_start = dash == nullptr ? range_start : dash + 1;
+ // Check for ranges with missing numbers, e.g. "", "3-", "-3".
+ if (num_start == pos || dash == range_start) return -1;
+ // The numbers are comprised only of digits, so it can only fail if it is out of
+ // range of int (the return type of this function).
+ unsigned long start_idx = strtoul(range_start, nullptr, 10);
+ if (start_idx > numeric_limits<int>::max()) return -1;
+ unsigned long end_idx = strtoul(num_start, nullptr, 10);
+ if (end_idx > numeric_limits<int>::max() || start_idx > end_idx) {
+ return -1;
+ }
+ // Keep track of the max index we've seen so far.
+ max_idx = std::max(static_cast<int>(end_idx), max_idx);
+ // End of line, expect no more input.
+ if (*pos == '\n') break;
+ ++pos;
+ }
+ // String must have a single newline at the very end.
+ if (*pos != '\n' || *(pos + 1) != '\0') return -1;
+ return max_idx;
}
#endif
diff --git a/src/kudu/gutil/sysinfo.h b/src/kudu/gutil/sysinfo.h
index 56f913c..d46cfe5 100644
--- a/src/kudu/gutil/sysinfo.h
+++ b/src/kudu/gutil/sysinfo.h
@@ -53,5 +53,17 @@ void SleepForMilliseconds(int64_t milliseconds);
// processor cycles per second of each processor. Thread-safe.
extern double CyclesPerSecond(void);
+// Parse the maximum CPU index from 'str'. The list is in the format of the CPU lists
+// under /sys/devices/system/cpu/, e.g. /sys/devices/system/cpu/present. Returns the
+// index of the max CPU or -1 if the string could not be parsed.
+// Examples of the format and the expected output include:
+// * "0\n" -> 0
+// * "0-8\n" -> 8
+// * "0-15,32-47\n" -> 47
+// * "2,4-127,128-143\n" -> 143
+// Ref: https://www.kernel.org/doc/Documentation/cputopology.txt
+// Exposed for testing.
+extern int ParseMaxCpuIndex(const char* str);
+
} // namespace base
#endif /* #ifndef _SYSINFO_H_ */