You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2019/04/09 15:48:56 UTC
[impala] 04/06: IMPALA-8395: Parse older formats of /proc/net/dev
correctly
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit d808bcda9df74d02d452bfa9f4ce491dd873f241
Author: Lars Volker <lv...@cloudera.com>
AuthorDate: Fri Apr 5 16:41:54 2019 -0700
IMPALA-8395: Parse older formats of /proc/net/dev correctly
Older kernel versions don't have a space between the interface name and
the first counter value in /proc/net/dev. This change reworks the
parsing logic to support such older formats and adds a unit test for it.
Change-Id: Ic804955d8e4269e787037a6dc68bef2d70382426
Reviewed-on: http://gerrit.cloudera.org:8080/12954
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/util/system-state-info-test.cc | 16 ++++++++++++++
be/src/util/system-state-info.cc | 40 +++++++++++++++++++++++++----------
be/src/util/system-state-info.h | 1 +
3 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/be/src/util/system-state-info-test.cc b/be/src/util/system-state-info-test.cc
index c80d577..c8680ba 100644
--- a/be/src/util/system-state-info-test.cc
+++ b/be/src/util/system-state-info-test.cc
@@ -89,6 +89,22 @@ TEST_F(SystemStateInfoTest, ParseProcNetDevString) {
EXPECT_EQ(state[SystemStateInfo::NET_TX_PACKETS], 176173628);
}
+// Tests parsing a string similar to the contents of /proc/net/dev on older kernels, such
+// as the one on Centos6.
+TEST_F(SystemStateInfoTest, ParseProcNetDevStringCentos6) {
+ // Fields are: user nice system idle iowait irq softirq steal guest guest_nice
+ string dev_net = R"(Inter-| Receive | Transmit
+ face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed
+ lo: 5829 53 0 0 0 0 0 0 5829 53 0 0 0 0 0 0
+ eth0:285025090 212208 0 0 0 0 0 0 9137793 84770 0 0 0 0 0 0)";
+ info.ReadProcNetDevString(dev_net);
+ const SystemStateInfo::NetworkValues& state = info.network_values_[info.net_val_idx_];
+ EXPECT_EQ(state[SystemStateInfo::NET_RX_BYTES], 285025090);
+ EXPECT_EQ(state[SystemStateInfo::NET_RX_PACKETS], 212208);
+ EXPECT_EQ(state[SystemStateInfo::NET_TX_BYTES], 9137793);
+ EXPECT_EQ(state[SystemStateInfo::NET_TX_PACKETS], 84770);
+}
+
// Tests that computing CPU ratios doesn't overflow
TEST_F(SystemStateInfoTest, ComputeCpuRatiosIntOverflow) {
// Load old and new values for CPU counters. These values are from a system where we
diff --git a/be/src/util/system-state-info.cc b/be/src/util/system-state-info.cc
index 400a063..1194bf4 100644
--- a/be/src/util/system-state-info.cc
+++ b/be/src/util/system-state-info.cc
@@ -16,9 +16,11 @@
// under the License.
#include "gutil/strings/split.h"
+#include "gutil/strings/strip.h"
#include "gutil/strings/util.h"
#include "kudu/util/env.h"
#include "kudu/util/faststring.h"
+#include "kudu/util/logging.h"
#include "util/error-util.h"
#include "util/string-parser.h"
#include "util/system-state-info.h"
@@ -35,6 +37,7 @@ using kudu::Env;
using kudu::faststring;
using kudu::ReadFileToString;
using strings::Split;
+using strings::SkipWhitespace;
namespace impala {
@@ -152,24 +155,39 @@ void SystemStateInfo::ReadProcNetDevString(const string& dev_string) {
void SystemStateInfo::ReadProcNetDevLine(
const string& dev_string, NetworkValues* result) {
- stringstream ss(dev_string);
+ StringPiece sp = StringPiece(dev_string);
+ // Lines can have leading whitespace
+ StripWhiteSpace(&sp);
- // Read the interface name
- string if_name;
- ss >> if_name;
+ // Initialize result to 0 to simplify error handling below
+ memset(result, 0, sizeof(*result));
// Don't include the loopback interface
- if (HasPrefixString(if_name, "lo:")) {
- memset(result, 0, sizeof(*result));
+ if (sp.starts_with("lo:")) return;
+
+ int colon_idx = sp.find_first_of(':');
+ if (colon_idx == StringPiece::npos) {
+ KLOG_EVERY_N_SECS(WARNING, 60) << "Failed to parse interface name in line: "
+ << sp.as_string();
+ return;
+ }
+
+ vector<StringPiece> counters = Split(sp.substr(colon_idx + 1), " ", SkipWhitespace());
+
+ // Something is wrong with the number of values that we read
+ if (NUM_NET_VALUES > counters.size()) {
+ KLOG_EVERY_N_SECS(WARNING, 60) << "Failed to parse enough values: expected "
+ << NUM_NET_VALUES << " but found " << counters.size();
return;
}
for (int i = 0; i < NUM_NET_VALUES; ++i) {
- int64_t v = -1;
- ss >> v;
- DCHECK_GE(v, 0) << "Value " << i << ": " << v;
- // Clamp invalid entries at 0
- (*result)[i] = max(v, 0L);
+ int64_t* v = &(*result)[i];
+ if (!safe_strto64(counters[i].as_string(), v)) {
+ KLOG_EVERY_N_SECS(WARNING, 60) << "Failed to parse value: " << *v;
+ *v = 0;
+ }
+ DCHECK_GE(*v, 0) << "Value " << i << ": " << *v;
}
}
diff --git a/be/src/util/system-state-info.h b/be/src/util/system-state-info.h
index 1b36e0c..e22d404 100644
--- a/be/src/util/system-state-info.h
+++ b/be/src/util/system-state-info.h
@@ -160,6 +160,7 @@ class SystemStateInfo {
FRIEND_TEST(SystemStateInfoTest, ComputeCpuRatiosIntOverflow);
FRIEND_TEST(SystemStateInfoTest, ComputeNetworkUsage);
FRIEND_TEST(SystemStateInfoTest, ParseProcNetDevString);
+ FRIEND_TEST(SystemStateInfoTest, ParseProcNetDevStringCentos6);
FRIEND_TEST(SystemStateInfoTest, ParseProcStat);
FRIEND_TEST(SystemStateInfoTest, ReadProcNetDev);
FRIEND_TEST(SystemStateInfoTest, ReadProcStat);