You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/05/29 15:34:06 UTC

[impala] branch master updated (7ea8c57 -> 396d577)

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.


    from 7ea8c57  IMPALA-5031: Out-of-range enum values are undefined behavior
     new c2aeb93  IMPALA-8560: Prometheus metrics support in Impala
     new 396d577  IMPALA-8585: Add tests for partitioned ACID tables

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:
 be/src/util/collection-metrics.h                   |  62 ++++
 be/src/util/histogram-metric.h                     |  77 ++++
 be/src/util/metrics-test.cc                        | 411 +++++++++++++++++++++
 be/src/util/metrics.cc                             |  55 +++
 be/src/util/metrics.h                              |  68 ++++
 .../java/org/apache/impala/util/AcidUtils.java     |   8 +
 .../java/org/apache/impala/util/AcidUtilsTest.java |  56 +++
 fe/src/test/resources/hive-site.xml.py             |   2 +-
 .../queries/QueryTest/acid-partitioned.test        | 133 +++++++
 tests/query_test/test_acid.py                      |   4 +
 tests/webserver/test_web_pages.py                  |   8 +
 11 files changed, 883 insertions(+), 1 deletion(-)
 create mode 100644 testdata/workloads/functional-query/queries/QueryTest/acid-partitioned.test


[impala] 01/02: IMPALA-8560: Prometheus metrics support in Impala

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit c2aeb93c4f5269e2a0ad2f027ef239767abd32dd
Author: Harshil <ha...@cloudera.com>
AuthorDate: Thu May 23 18:37:48 2019 -0700

    IMPALA-8560: Prometheus metrics support in Impala
    
        -- This change adds Prometheus text explosion format metric
           generation.
        -- More details can be found below:
        -- https://prometheus.io/docs/instrumenting/exposition_formats
        -- Added unit test to test this change
    
    Tests:
        -- Feed all this metrics to prometheus running on local host
        -- Also ran it against a "./promtool" to check for any error in
           metrics format for prometheus.
    Change-Id: I5349085a2007b568cb97f9b8130804ea64d7bb08
    Reviewed-on: http://gerrit.cloudera.org:8080/13345
    Reviewed-by: Tim Armstrong <ta...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/util/collection-metrics.h  |  62 ++++++
 be/src/util/histogram-metric.h    |  77 +++++++
 be/src/util/metrics-test.cc       | 411 ++++++++++++++++++++++++++++++++++++++
 be/src/util/metrics.cc            |  55 +++++
 be/src/util/metrics.h             |  68 +++++++
 tests/webserver/test_web_pages.py |   8 +
 6 files changed, 681 insertions(+)

diff --git a/be/src/util/collection-metrics.h b/be/src/util/collection-metrics.h
index dc7fb54..4cf3330 100644
--- a/be/src/util/collection-metrics.h
+++ b/be/src/util/collection-metrics.h
@@ -74,6 +74,12 @@ class SetMetric : public Metric {
 
   void Reset() { value_.clear(); }
 
+  virtual TMetricKind::type ToPrometheus(
+      std::string name, std::stringstream* val, std::stringstream* metric_kind) {
+    // this is not supported type in prometheus, so ignore
+    return TMetricKind::SET;
+  }
+
   virtual void ToJson(rapidjson::Document* document, rapidjson::Value* value) {
     rapidjson::Value container(rapidjson::kObjectType);
     AddStandardFields(document, &container);
@@ -157,6 +163,62 @@ class StatsMetric : public Metric {
     acc_ = Accumulator();
   }
 
+  virtual TMetricKind::type ToPrometheus(
+      std::string name, std::stringstream* val, std::stringstream* metric_kind) {
+    boost::lock_guard<boost::mutex> l(lock_);
+
+    *val << name << "_total " << boost::accumulators::count(acc_) << "\n";
+
+    if (boost::accumulators::count(acc_) > 0) {
+      if (IsUnitTimeBased(unit_)) {
+        *val << name << "_last " << ConvertToPrometheusSecs(value_, unit_) << "\n";
+      } else {
+        *val << name << "_last " << value_ << "\n";
+      }
+
+      if (StatsSelection & StatsType::MIN) {
+        if (IsUnitTimeBased(unit_)) {
+          *val << name << "_min "
+               << ConvertToPrometheusSecs(boost::accumulators::min(acc_), unit_) << "\n";
+        } else {
+          *val << name << "_min " << boost::accumulators::min(acc_) << "\n";
+        }
+      }
+
+      if (StatsSelection & StatsType::MAX) {
+        if (IsUnitTimeBased(unit_)) {
+          *val << name << "_max "
+               << ConvertToPrometheusSecs(boost::accumulators::max(acc_), unit_) << "\n";
+        } else {
+          *val << name << "_max " << boost::accumulators::max(acc_) << "\n";
+        }
+      }
+
+      if (StatsSelection & StatsType::MEAN) {
+        if (IsUnitTimeBased(unit_)) {
+          *val << name << "_mean "
+               << ConvertToPrometheusSecs(boost::accumulators::mean(acc_), unit_) << "\n";
+        } else {
+          *val << name << "_mean " << boost::accumulators::mean(acc_) << "\n";
+        }
+      }
+
+      if (StatsSelection & StatsType::STDDEV) {
+        if (IsUnitTimeBased(unit_)) {
+          *val << name << "_stddev "
+               << ConvertToPrometheusSecs(
+                      std::sqrt(boost::accumulators::variance(acc_)), unit_)
+               << "\n";
+        } else {
+          *val << name << "_stddev " << std::sqrt(boost::accumulators::variance(acc_))
+               << "\n";
+        }
+      }
+    }
+    *metric_kind << "# TYPE " << name << " counter";
+    return TMetricKind::STATS;
+  }
+
   virtual void ToJson(rapidjson::Document* document, rapidjson::Value* val) {
     boost::lock_guard<boost::mutex> l(lock_);
     rapidjson::Value container(rapidjson::kObjectType);
diff --git a/be/src/util/histogram-metric.h b/be/src/util/histogram-metric.h
index 43d4eaf..ca4499f 100644
--- a/be/src/util/histogram-metric.h
+++ b/be/src/util/histogram-metric.h
@@ -71,6 +71,83 @@ class HistogramMetric : public Metric {
     *value = container;
   }
 
+  virtual TMetricKind::type ToPrometheus(std::string name, std::stringstream* value,
+      std::stringstream* metric_kind) override {
+    {
+      boost::lock_guard<SpinLock> l(lock_);
+
+      // check if unit its 'TIME_MS','TIME_US' or 'TIME_NS' and convert it to seconds,
+      // this is because prometheus only supports time format in seconds
+      if (IsUnitTimeBased(unit_)) {
+        *value << name << "{le=\"0.2\"} "
+               << ConvertToPrometheusSecs(histogram_->ValueAtPercentile(25), unit_)
+               << "\n";
+      } else {
+        *value << name << "{le=\"0.2\"} " << histogram_->ValueAtPercentile(25) << "\n";
+      }
+
+      if (IsUnitTimeBased(unit_)) {
+        *value << name << "{le=\"0.5\"} "
+               << ConvertToPrometheusSecs(histogram_->ValueAtPercentile(50), unit_)
+               << "\n";
+      } else {
+        *value << name << "{le=\"0.5\"} " << histogram_->ValueAtPercentile(50) << "\n";
+      }
+
+      if (IsUnitTimeBased(unit_)) {
+        *value << name << "{le=\"0.7\"} "
+               << ConvertToPrometheusSecs(histogram_->ValueAtPercentile(75), unit_)
+               << "\n";
+      } else {
+        *value << name << "{le=\"0.7\"} " << histogram_->ValueAtPercentile(75) << "\n";
+      }
+
+      if (IsUnitTimeBased(unit_)) {
+        *value << name << "{le=\"0.9\"} "
+               << ConvertToPrometheusSecs(histogram_->ValueAtPercentile(90), unit_)
+               << "\n";
+      } else {
+        *value << name << "{le=\"0.9\"} " << histogram_->ValueAtPercentile(90) << "\n";
+      }
+
+      if (IsUnitTimeBased(unit_)) {
+        *value << name << "{le=\"0.95\"} "
+               << ConvertToPrometheusSecs(histogram_->ValueAtPercentile(95), unit_)
+               << "\n";
+      } else {
+        *value << name << "{le=\"0.95\"} " << histogram_->ValueAtPercentile(95) << "\n";
+      }
+
+      if (IsUnitTimeBased(unit_)) {
+        *value << name << "{le=\"0.999\"} "
+               << ConvertToPrometheusSecs(histogram_->ValueAtPercentile(99.9), unit_)
+               << "\n";
+      } else {
+        *value << name << "{le=\"0.999\"} " << histogram_->ValueAtPercentile(99.9)
+               << "\n";
+      }
+
+      if (IsUnitTimeBased(unit_)) {
+        *value << name << "_max "
+               << ConvertToPrometheusSecs(histogram_->MaxValue(), unit_) << "\n";
+      } else {
+        *value << name << "_max " << histogram_->MaxValue() << "\n";
+      }
+
+      if (IsUnitTimeBased(unit_)) {
+        *value << name << "_min "
+               << ConvertToPrometheusSecs(histogram_->MinValue(), unit_) << "\n";
+      } else {
+        *value << name << "_min " << histogram_->MinValue() << "\n";
+      }
+
+      *value << name << "_count " << histogram_->TotalCount();
+    }
+
+    *metric_kind << "# TYPE " << name << " histogram";
+    return TMetricKind::HISTOGRAM;
+  }
+
   void Update(int64_t val) {
     boost::lock_guard<SpinLock> l(lock_);
     histogram_->Increment(val);
diff --git a/be/src/util/metrics-test.cc b/be/src/util/metrics-test.cc
index accbdcd..3302fb3 100644
--- a/be/src/util/metrics-test.cc
+++ b/be/src/util/metrics-test.cc
@@ -463,5 +463,416 @@ TEST_F(MetricsTest, MetricGroupJson) {
   EXPECT_EQ(val2["name"].GetString(), string("child1"));
 }
 
+void AssertPrometheus(const std::stringstream& val, const string& name,
+    const string& value, const string& desc, const string& kind = "") {
+  std::stringstream exp_val;
+  // convert to all values to expected format
+  exp_val << "# HELP " << name << " " << desc << "\n"
+          << "# TYPE " << name << " " << kind << "\n";
+  if (name == "stats_metric" || name == "histogram_metric") {
+    exp_val << value + "\n";
+  } else {
+    exp_val << name << " " << value + "\n";
+  }
+  EXPECT_EQ(val.str(), exp_val.str());
+}
+
+TEST_F(MetricsTest, CountersPrometheus) {
+  MetricGroup metrics("CounterMetrics");
+  AddMetricDef("counter", TMetricKind::COUNTER, TUnit::UNIT, "description");
+  metrics.AddCounter("counter", 0);
+  std::stringstream counter_val;
+  metrics.ToPrometheus(true, &counter_val);
+  AssertPrometheus(counter_val, "counter", "0", "description", "counter");
+}
+
+TEST_F(MetricsTest, CountersBytesPrometheus) {
+  MetricGroup metrics("CounterMetrics");
+  AddMetricDef("counter", TMetricKind::COUNTER, TUnit::BYTES, "description");
+  metrics.AddCounter("counter", 555);
+  std::stringstream counter_val;
+  metrics.ToPrometheus(true, &counter_val);
+  AssertPrometheus(counter_val, "counter", "555", "description", "counter");
+}
+
+TEST_F(MetricsTest, CountersNonePrometheus) {
+  MetricGroup metrics("CounterMetrics");
+  AddMetricDef("counter", TMetricKind::COUNTER, TUnit::NONE, "description");
+  metrics.AddCounter("counter", 0);
+  std::stringstream counter_val;
+  metrics.ToPrometheus(true, &counter_val);
+  AssertPrometheus(counter_val, "counter", "0", "description", "counter");
+}
+
+TEST_F(MetricsTest, CountersTimeMSPrometheus) {
+  MetricGroup metrics("CounterMetrics");
+  AddMetricDef("counter", TMetricKind::COUNTER, TUnit::TIME_MS, "description");
+  metrics.AddCounter("counter", 4354364);
+  std::stringstream counter_val;
+  metrics.ToPrometheus(true, &counter_val);
+  AssertPrometheus(counter_val, "counter", "4354.36", "description", "counter");
+}
+
+TEST_F(MetricsTest, CountersTimeNSPrometheus) {
+  MetricGroup metrics("CounterMetrics");
+  AddMetricDef("counter", TMetricKind::COUNTER, TUnit::TIME_NS, "description");
+  metrics.AddCounter("counter", 4354364234);
+  std::stringstream counter_val;
+  metrics.ToPrometheus(true, &counter_val);
+  AssertPrometheus(counter_val, "counter", "4.35436", "description", "counter");
+}
+
+TEST_F(MetricsTest, CountersTimeSPrometheus) {
+  MetricGroup metrics("CounterMetrics");
+  AddMetricDef("counter", TMetricKind::COUNTER, TUnit::TIME_S, "description");
+  metrics.AddCounter("counter", 120);
+  std::stringstream counter_val;
+  metrics.ToPrometheus(true, &counter_val);
+  AssertPrometheus(counter_val, "counter", "120", "description", "counter");
+}
+
+TEST_F(MetricsTest, GaugesPrometheus) {
+  MetricGroup metrics("GaugeMetrics");
+  AddMetricDef("gauge", TMetricKind::GAUGE, TUnit::NONE);
+  metrics.AddGauge("gauge", 10);
+  std::stringstream gauge_val;
+  metrics.ToPrometheus(true, &gauge_val);
+  AssertPrometheus(gauge_val, "gauge", "10", "", "gauge");
+}
+
+TEST_F(MetricsTest, GaugesBytesPrometheus) {
+  MetricGroup metrics("GaugeMetrics");
+  AddMetricDef("gauge", TMetricKind::GAUGE, TUnit::BYTES);
+  metrics.AddGauge("gauge", 150000);
+  std::stringstream gauge_val;
+  metrics.ToPrometheus(true, &gauge_val);
+  AssertPrometheus(gauge_val, "gauge", "150000", "", "gauge");
+}
+
+TEST_F(MetricsTest, GaugesTimeMSPrometheus) {
+  MetricGroup metrics("GaugeMetrics");
+  AddMetricDef("gauge", TMetricKind::GAUGE, TUnit::TIME_MS);
+  metrics.AddGauge("gauge", 10000);
+  std::stringstream gauge_val;
+  metrics.ToPrometheus(true, &gauge_val);
+  AssertPrometheus(gauge_val, "gauge", "10", "", "gauge");
+}
+
+TEST_F(MetricsTest, GaugesTimeNSPrometheus) {
+  MetricGroup metrics("GaugeMetrics");
+  AddMetricDef("gauge", TMetricKind::GAUGE, TUnit::TIME_NS);
+  metrics.AddGauge("gauge", 2334123456);
+  std::stringstream gauge_val;
+  metrics.ToPrometheus(true, &gauge_val);
+  AssertPrometheus(gauge_val, "gauge", "2.33412", "", "gauge");
+}
+
+TEST_F(MetricsTest, GaugesTimeSPrometheus) {
+  MetricGroup metrics("GaugeMetrics");
+  AddMetricDef("gauge", TMetricKind::GAUGE, TUnit::TIME_S);
+  metrics.AddGauge("gauge", 1500);
+  std::stringstream gauge_val;
+  metrics.ToPrometheus(true, &gauge_val);
+  AssertPrometheus(gauge_val, "gauge", "1500", "", "gauge");
+}
+
+TEST_F(MetricsTest, GaugesUnitPrometheus) {
+  MetricGroup metrics("GaugeMetrics");
+  AddMetricDef("gauge", TMetricKind::GAUGE, TUnit::UNIT);
+  metrics.AddGauge("gauge", 111);
+  std::stringstream gauge_val;
+  metrics.ToPrometheus(true, &gauge_val);
+  AssertPrometheus(gauge_val, "gauge", "111", "", "gauge");
+}
+
+TEST_F(MetricsTest, StatsMetricsPrometheus) {
+  MetricGroup metrics("StatsMetrics");
+  AddMetricDef("stats_metric", TMetricKind::STATS, TUnit::UNIT);
+  StatsMetric<double>* metric =
+      StatsMetric<double>::CreateAndRegister(&metrics, "stats_metric");
+  metric->Update(10.0);
+  metric->Update(20.0);
+  std::stringstream stats_val;
+  metrics.ToPrometheus(true, &stats_val);
+  AssertPrometheus(stats_val, "stats_metric",
+      "stats_metric_total 2\n"
+      "stats_metric_last 20\n"
+      "stats_metric_min 10\n"
+      "stats_metric_max 20\n"
+      "stats_metric_mean 15\n"
+      "stats_metric_stddev 5\n",
+      "", "counter");
+}
+
+TEST_F(MetricsTest, StatsMetricsBytesPrometheus) {
+  MetricGroup metrics("StatsMetrics");
+  AddMetricDef("stats_metric", TMetricKind::STATS, TUnit::BYTES);
+  StatsMetric<double>* metric =
+      StatsMetric<double>::CreateAndRegister(&metrics, "stats_metric");
+  metric->Update(10.0);
+  metric->Update(2230.1234567);
+  std::stringstream stats_val;
+  metrics.ToPrometheus(true, &stats_val);
+  AssertPrometheus(stats_val, "stats_metric",
+      "stats_metric_total 2\n"
+      "stats_metric_last 2230.12\n"
+      "stats_metric_min 10\n"
+      "stats_metric_max 2230.12\n"
+      "stats_metric_mean 1120.06\n"
+      "stats_metric_stddev 1110.06\n",
+      "", "counter");
+}
+
+TEST_F(MetricsTest, StatsMetricsNonePrometheus) {
+  MetricGroup metrics("StatsMetrics");
+  AddMetricDef("stats_metric", TMetricKind::STATS, TUnit::NONE);
+  StatsMetric<double>* metric =
+      StatsMetric<double>::CreateAndRegister(&metrics, "stats_metric");
+  metric->Update(10.0);
+  metric->Update(20.0);
+  std::stringstream stats_val;
+  metrics.ToPrometheus(true, &stats_val);
+  AssertPrometheus(stats_val, "stats_metric",
+      "stats_metric_total 2\n"
+      "stats_metric_last 20\n"
+      "stats_metric_min 10\n"
+      "stats_metric_max 20\n"
+      "stats_metric_mean 15\n"
+      "stats_metric_stddev 5\n",
+      "", "counter");
+}
+
+TEST_F(MetricsTest, StatsMetricsTimeMSPrometheus) {
+  MetricGroup metrics("StatsMetrics");
+  AddMetricDef("stats_metric", TMetricKind::STATS, TUnit::TIME_MS);
+  StatsMetric<double>* metric =
+      StatsMetric<double>::CreateAndRegister(&metrics, "stats_metric");
+  metric->Update(10.0);
+  metric->Update(20.0);
+  std::stringstream stats_val;
+  metrics.ToPrometheus(true, &stats_val);
+  AssertPrometheus(stats_val, "stats_metric",
+      "stats_metric_total 2\n"
+      "stats_metric_last 0.02\n"
+      "stats_metric_min 0.01\n"
+      "stats_metric_max 0.02\n"
+      "stats_metric_mean 0.015\n"
+      "stats_metric_stddev 0.005\n",
+      "", "counter");
+}
+
+TEST_F(MetricsTest, StatsMetricsTimeNSPrometheus) {
+  MetricGroup metrics("StatsMetrics");
+  AddMetricDef("stats_metric", TMetricKind::STATS, TUnit::TIME_NS);
+  StatsMetric<double>* metric =
+      StatsMetric<double>::CreateAndRegister(&metrics, "stats_metric");
+  metric->Update(10.12345);
+  metric->Update(20.567);
+  std::stringstream stats_val;
+  metrics.ToPrometheus(true, &stats_val);
+  AssertPrometheus(stats_val, "stats_metric",
+      "stats_metric_total 2\n"
+      "stats_metric_last 2.0567e-08\n"
+      "stats_metric_min 1.01235e-08\n"
+      "stats_metric_max 2.0567e-08\n"
+      "stats_metric_mean 1.53452e-08\n"
+      "stats_metric_stddev 5.22178e-09\n",
+      "", "counter");
+}
+
+TEST_F(MetricsTest, StatsMetricsTimeSPrometheus) {
+  MetricGroup metrics("StatsMetrics");
+  AddMetricDef("stats_metric", TMetricKind::STATS, TUnit::TIME_S);
+  StatsMetric<double>* metric =
+      StatsMetric<double>::CreateAndRegister(&metrics, "stats_metric");
+  metric->Update(10.22);
+  metric->Update(20.22);
+  std::stringstream stats_val;
+  metrics.ToPrometheus(true, &stats_val);
+  AssertPrometheus(stats_val, "stats_metric",
+      "stats_metric_total 2\n"
+      "stats_metric_last 20.22\n"
+      "stats_metric_min 10.22\n"
+      "stats_metric_max 20.22\n"
+      "stats_metric_mean 15.22\n"
+      "stats_metric_stddev 5\n",
+      "", "counter");
+}
+
+TEST_F(MetricsTest, HistogramPrometheus) {
+  MetricGroup metrics("HistoMetrics");
+  TMetricDef metric_def =
+      MakeTMetricDef("histogram-metric", TMetricKind::HISTOGRAM, TUnit::TIME_MS);
+  constexpr int MAX_VALUE = 10000;
+  HistogramMetric* metric =
+      metrics.RegisterMetric(new HistogramMetric(metric_def, MAX_VALUE, 3));
+
+  // Add value beyond limit to make sure it's recorded accurately.
+  for (int i = 0; i <= MAX_VALUE + 1; ++i) metric->Update(i);
+
+  std::stringstream val;
+  metrics.ToPrometheus(true, &val);
+  AssertPrometheus(val, "histogram_metric",
+      "histogram_metric{le=\"0.2\"} 2.5\n"
+      "histogram_metric{le=\"0.5\"} 5\n"
+      "histogram_metric{le=\"0.7\"} 7.5\n"
+      "histogram_metric{le=\"0.9\"} 9\n"
+      "histogram_metric{le=\"0.95\"} 9.496\n"
+      "histogram_metric{le=\"0.999\"} 9.984\n"
+      "histogram_metric_max 10.001\n"
+      "histogram_metric_min 0\n"
+      "histogram_metric_count 10002",
+      "", "histogram");
+}
+
+TEST_F(MetricsTest, HistogramTimeNSPrometheus) {
+  MetricGroup metrics("HistoMetrics");
+  TMetricDef metric_def =
+      MakeTMetricDef("histogram-metric", TMetricKind::HISTOGRAM, TUnit::TIME_NS);
+  constexpr int MAX_VALUE = 10000;
+  HistogramMetric* metric =
+      metrics.RegisterMetric(new HistogramMetric(metric_def, MAX_VALUE, 3));
+
+  // Add value beyond limit to make sure it's recorded accurately.
+  for (int i = 0; i <= MAX_VALUE + 1; ++i) metric->Update(i);
+
+  std::stringstream val;
+  metrics.ToPrometheus(true, &val);
+  AssertPrometheus(val, "histogram_metric",
+      "histogram_metric{le=\"0.2\"} 2.5e-06\n"
+      "histogram_metric{le=\"0.5\"} 5e-06\n"
+      "histogram_metric{le=\"0.7\"} 7.5e-06\n"
+      "histogram_metric{le=\"0.9\"} 9e-06\n"
+      "histogram_metric{le=\"0.95\"} 9.496e-06\n"
+      "histogram_metric{le=\"0.999\"} 9.984e-06\n"
+      "histogram_metric_max 1.0001e-05\n"
+      "histogram_metric_min 0\n"
+      "histogram_metric_count 10002",
+      "", "histogram");
+}
+
+TEST_F(MetricsTest, HistogramTimeSPrometheus) {
+  MetricGroup metrics("HistoMetrics");
+  TMetricDef metric_def =
+      MakeTMetricDef("histogram-metric", TMetricKind::HISTOGRAM, TUnit::TIME_S);
+  constexpr int MAX_VALUE = 10000;
+  HistogramMetric* metric =
+      metrics.RegisterMetric(new HistogramMetric(metric_def, MAX_VALUE, 3));
+
+  // Add value beyond limit to make sure it's recorded accurately.
+  for (int i = 0; i <= MAX_VALUE + 1; ++i) metric->Update(i);
+
+  std::stringstream val;
+  metrics.ToPrometheus(true, &val);
+  AssertPrometheus(val, "histogram_metric",
+      "histogram_metric{le=\"0.2\"} 2500\n"
+      "histogram_metric{le=\"0.5\"} 5000\n"
+      "histogram_metric{le=\"0.7\"} 7500\n"
+      "histogram_metric{le=\"0.9\"} 9000\n"
+      "histogram_metric{le=\"0.95\"} 9496\n"
+      "histogram_metric{le=\"0.999\"} 9984\n"
+      "histogram_metric_max 10001\n"
+      "histogram_metric_min 0\n"
+      "histogram_metric_count 10002",
+      "", "histogram");
+}
+
+TEST_F(MetricsTest, HistogramBytesPrometheus) {
+  MetricGroup metrics("HistoMetrics");
+  TMetricDef metric_def =
+      MakeTMetricDef("histogram-metric", TMetricKind::HISTOGRAM, TUnit::BYTES);
+  constexpr int MAX_VALUE = 10000;
+  HistogramMetric* metric =
+      metrics.RegisterMetric(new HistogramMetric(metric_def, MAX_VALUE, 3));
+
+  // Add value beyond limit to make sure it's recorded accurately.
+  for (int i = 0; i <= MAX_VALUE + 1; ++i) metric->Update(i);
+
+  std::stringstream val;
+  metrics.ToPrometheus(true, &val);
+  AssertPrometheus(val, "histogram_metric",
+      "histogram_metric{le=\"0.2\"} 2500\n"
+      "histogram_metric{le=\"0.5\"} 5000\n"
+      "histogram_metric{le=\"0.7\"} 7500\n"
+      "histogram_metric{le=\"0.9\"} 9000\n"
+      "histogram_metric{le=\"0.95\"} 9496\n"
+      "histogram_metric{le=\"0.999\"} 9984\n"
+      "histogram_metric_max 10001\n"
+      "histogram_metric_min 0\n"
+      "histogram_metric_count 10002",
+      "", "histogram");
+}
+
+TEST_F(MetricsTest, HistogramUnitPrometheus) {
+  MetricGroup metrics("HistoMetrics");
+  TMetricDef metric_def =
+      MakeTMetricDef("histogram-metric", TMetricKind::HISTOGRAM, TUnit::UNIT);
+  constexpr int MAX_VALUE = 10000;
+  HistogramMetric* metric =
+      metrics.RegisterMetric(new HistogramMetric(metric_def, MAX_VALUE, 3));
+
+  // Add value beyond limit to make sure it's recorded accurately.
+  for (int i = 0; i <= MAX_VALUE + 1; ++i) metric->Update(i);
+
+  std::stringstream val;
+  metrics.ToPrometheus(true, &val);
+  AssertPrometheus(val, "histogram_metric",
+      "histogram_metric{le=\"0.2\"} 2500\n"
+      "histogram_metric{le=\"0.5\"} 5000\n"
+      "histogram_metric{le=\"0.7\"} 7500\n"
+      "histogram_metric{le=\"0.9\"} 9000\n"
+      "histogram_metric{le=\"0.95\"} 9496\n"
+      "histogram_metric{le=\"0.999\"} 9984\n"
+      "histogram_metric_max 10001\n"
+      "histogram_metric_min 0\n"
+      "histogram_metric_count 10002",
+      "", "histogram");
+}
+
+TEST_F(MetricsTest, MetricGroupPrometheus) {
+  std::stringstream exp_val;
+  exp_val << "# HELP counter1 description\n"
+             "# TYPE counter1 counter\n"
+             "counter1 2048\n"
+             "# HELP counter2 description\n"
+             "# TYPE counter2 counter\n"
+             "counter2 2048\n"
+             "# HELP child_counter description\n"
+             "# TYPE child_counter counter\n"
+             "child_counter 0\n";
+  MetricGroup metrics("PrometheusTest");
+  AddMetricDef("counter1", TMetricKind::COUNTER, TUnit::BYTES, "description");
+  AddMetricDef("counter2", TMetricKind::COUNTER, TUnit::BYTES, "description");
+  metrics.AddCounter("counter1", 2048);
+  metrics.AddCounter("counter2", 2048);
+
+  MetricGroup* find_result = metrics.FindChildGroup("child1");
+  EXPECT_EQ(find_result, reinterpret_cast<MetricGroup*>(NULL));
+
+  metrics.GetOrCreateChildGroup("child1");
+  AddMetricDef("child_counter", TMetricKind::COUNTER, TUnit::BYTES, "description");
+  metrics.GetOrCreateChildGroup("child2")->AddCounter("child_counter", 0);
+
+  IntCounter* counter = metrics.FindMetricForTesting<IntCounter>(string("child_counter"));
+  ASSERT_NE(counter, reinterpret_cast<IntCounter*>(NULL));
+
+  std::stringstream val;
+  metrics.ToPrometheus(true, &val);
+  EXPECT_EQ(val.str(), exp_val.str());
+}
+
+// test with null metrics
+TEST_F(MetricsTest, StatsMetricsNullPrometheus) {
+  MetricGroup nullMetrics("StatsMetrics");
+  AddMetricDef("", TMetricKind::STATS, TUnit::TIME_S);
+  std::stringstream stats_val;
+  nullMetrics.ToPrometheus(true, &stats_val);
+  EXPECT_EQ("", stats_val.str());
+
+  MetricGroup metrics("Metrics");
+  AddMetricDef("test", TMetricKind::STATS, TUnit::TIME_S);
+  metrics.ToPrometheus(true, &stats_val);
+  EXPECT_EQ("", stats_val.str());
+}
 }
 
diff --git a/be/src/util/metrics.cc b/be/src/util/metrics.cc
index 34e5018..2a9cbf6 100644
--- a/be/src/util/metrics.cc
+++ b/be/src/util/metrics.cc
@@ -31,6 +31,7 @@
 
 #include "common/names.h"
 
+using boost::algorithm::replace_all_copy;
 using namespace impala;
 using namespace rapidjson;
 using namespace strings;
@@ -91,6 +92,10 @@ Status MetricGroup::Init(Webserver* webserver) {
     Webserver::UrlCallback json_callback =
         bind<void>(mem_fn(&MetricGroup::TemplateCallback), this, _1, _2);
     webserver->RegisterUrlCallback("/metrics", "metrics.tmpl", json_callback, true);
+
+    Webserver::RawUrlCallback prometheus_callback =
+        bind<void>(mem_fn(&MetricGroup::PrometheusCallback), this, _1, _2);
+    webserver->RegisterUrlCallback("/metrics_prometheus", prometheus_callback);
   }
 
   return Status::OK();
@@ -172,6 +177,20 @@ void MetricGroup::TemplateCallback(const Webserver::WebRequest& req,
   }
 }
 
+void MetricGroup::PrometheusCallback(
+    const Webserver::WebRequest& req, stringstream* data) {
+  const auto& args = req.parsed_args;
+  Webserver::ArgumentMap::const_iterator metric_group = args.find("metric_group");
+
+  lock_guard<SpinLock> l(lock_);
+  // If no particular metric group is requested, render this metric group (and all its
+  // children).
+  if (metric_group == args.end()) {
+    Value container;
+    ToPrometheus(true, data);
+  }
+}
+
 void MetricGroup::ToJson(bool include_children, Document* document, Value* out_val) {
   Value metric_list(kArrayType);
   for (const MetricMap::value_type& m: metric_map_) {
@@ -197,6 +216,42 @@ void MetricGroup::ToJson(bool include_children, Document* document, Value* out_v
   *out_val = container;
 }
 
+void MetricGroup::ToPrometheus(bool include_children, stringstream* out_val) {
+  for (auto const& m : metric_map_) {
+    stringstream metric_value;
+    stringstream metric_kind;
+
+    // replace all occurrence of '.' and '-'
+    string name = replace_all_copy(m.first, ".", "_");
+    name = replace_all_copy(name, "-", "_");
+    TMetricKind::type metric_type =
+        m.second->ToPrometheus(name, &metric_value, &metric_kind);
+    if (metric_type == TMetricKind::SET || metric_type == TMetricKind::PROPERTY) {
+      // not supported in prometheus
+      continue;
+    }
+    *out_val << "# HELP " << name << " ";
+    *out_val << m.second->description_;
+    *out_val << "\n";
+    *out_val << metric_kind.str();
+    *out_val << "\n";
+    // append only if metric type is not stats, set or histogram
+    if (metric_type != TMetricKind::HISTOGRAM && metric_type != TMetricKind::STATS) {
+      *out_val << name;
+      *out_val << " ";
+    }
+    *out_val << metric_value.str();
+    *out_val << "\n";
+  }
+
+  if (include_children) {
+    Value child_groups(kArrayType);
+    for (const ChildGroupMap::value_type& child : children_) {
+      child.second->ToPrometheus(true, out_val);
+    }
+  }
+}
+
 MetricGroup* MetricGroup::GetOrCreateChildGroup(const string& name) {
   lock_guard<SpinLock> l(lock_);
   ChildGroupMap::iterator it = children_.find(name);
diff --git a/be/src/util/metrics.h b/be/src/util/metrics.h
index 80f899d..b319f35 100644
--- a/be/src/util/metrics.h
+++ b/be/src/util/metrics.h
@@ -96,6 +96,16 @@ class Metric {
   /// This method is kept for backwards-compatibility with CM5.0.
   virtual void ToLegacyJson(rapidjson::Document* document) = 0;
 
+  /// Builds a new Value into 'val', based on prometheus text exposition format
+  /// Details of this format can be found below:
+  /// https://github.com/prometheus/docs/blob/master/content/docs/instrumenting/
+  //  exposition_formats.md
+  /// Should set the following fields where appropriate:
+  //
+  /// name, value, metric_kind
+  virtual TMetricKind::type ToPrometheus(
+      string name, std::stringstream* val, std::stringstream* metric_kind) = 0;
+
   /// Writes a human-readable representation of this metric to 'out'. This is the
   /// representation that is often displayed in webpages etc.
   virtual std::string ToHumanReadable() = 0;
@@ -103,6 +113,11 @@ class Metric {
   const std::string& key() const { return key_; }
   const std::string& description() const { return description_; }
 
+  bool IsUnitTimeBased(TUnit::type type) {
+    return (type == TUnit::type::TIME_MS || type == TUnit::type::TIME_US
+        || type == TUnit::type::TIME_NS);
+  }
+
  protected:
   /// Unique key identifying this metric
   const std::string key_;
@@ -120,6 +135,26 @@ class Metric {
   void AddStandardFields(rapidjson::Document* document, rapidjson::Value* val);
 };
 
+template <typename T>
+inline double ConvertToPrometheusSecs(const T& val, TUnit::type unit) {
+  double value = val;
+  if (unit == TUnit::type::TIME_MS) {
+    value /= 1000;
+  } else if (unit == TUnit::type::TIME_US) {
+    value /= 1000000;
+  } else if (unit == TUnit::type::TIME_NS) {
+    value /= 1000000000;
+  }
+  return value;
+}
+
+template <>
+inline double ConvertToPrometheusSecs<std::string>(
+    const std::string& val, TUnit::type unit) {
+  DCHECK(false) << "Should not be called for string metrics";
+  return 0.0;
+}
+
 /// A ScalarMetric has a value which is a simple primitive type: e.g. integers, strings
 /// and floats. It is parameterised not only by the type of its value, but by both the
 /// unit (e.g. bytes/s), drawn from TUnit and the 'kind' of the metric itself.
@@ -160,6 +195,30 @@ class ScalarMetric: public Metric {
     *val = container;
   }
 
+  virtual TMetricKind::type ToPrometheus(
+      std::string name, std::stringstream* val, std::stringstream* metric_kind) override {
+    std::string metric_type = PrintThriftEnum(kind()).c_str();
+    // prometheus doesn't support 'property', so ignore it
+    if (!metric_type.compare("property")) {
+      return TMetricKind::PROPERTY;
+    }
+
+    if (IsUnitTimeBased(unit())) {
+      // check if unit its 'TIME_MS','TIME_US' or 'TIME_NS' and convert it to seconds,
+      // this is because prometheus only supports time format in seconds
+      *val << ConvertToPrometheusSecs(GetValue(), unit());
+    } else {
+      *val << GetValue();
+    }
+
+    // convert metric type to lower case, that's what prometheus expects
+    std::transform(
+        metric_type.begin(), metric_type.end(), metric_type.begin(), ::tolower);
+
+    *metric_kind << "# TYPE " << name << " " << metric_type;
+    return kind();
+  }
+
   virtual std::string ToHumanReadable() override {
     return PrettyPrinter::Print(GetValue(), unit());
   }
@@ -440,6 +499,9 @@ class MetricGroup {
   void ToJson(bool include_children, rapidjson::Document* document,
       rapidjson::Value* out_val);
 
+  /// Converts this metric group (and optionally all of its children recursively) to JSON.
+  void ToPrometheus(bool include_children, std::stringstream* out_val);
+
   /// Creates or returns an already existing child metric group.
   MetricGroup* GetOrCreateChildGroup(const std::string& name);
 
@@ -476,6 +538,12 @@ class MetricGroup {
   void TemplateCallback(const Webserver::WebRequest& req,
       rapidjson::Document* document);
 
+  /// Webserver callback for /metricsPrometheus. Produces string in prometheus format,
+  /// each representing metric group, and each including a list of metrics, and a list
+  /// of immediate children.  If args contains a paramater 'metric', only the json for
+  /// that metric is returned.
+  void PrometheusCallback(const Webserver::WebRequest& req, std::stringstream* data);
+
   /// Legacy webpage callback for CM 5.0 and earlier. Produces a flattened map of (key,
   /// value) pairs for all metrics in this hierarchy.
   /// If args contains a paramater 'metric', only the json for that metric is returned.
diff --git a/tests/webserver/test_web_pages.py b/tests/webserver/test_web_pages.py
index 581df17..9dd3405 100644
--- a/tests/webserver/test_web_pages.py
+++ b/tests/webserver/test_web_pages.py
@@ -46,6 +46,7 @@ class TestWebPage(ImpalaTestSuite):
   ADMISSION_URL = "http://localhost:{0}/admission"
   RESET_RESOURCE_POOL_STATS_URL = "http://localhost:{0}/resource_pool_reset"
   BACKENDS_URL = "http://localhost:{0}/backends"
+  PROMETHEUS_METRICS_URL = "http://localhost:{0}/metrics_prometheus"
 
   # log4j changes do not apply to the statestore since it doesn't
   # have an embedded JVM. So we make two sets of ports to test the
@@ -564,3 +565,10 @@ class TestWebPage(ImpalaTestSuite):
     # Check the query id is in the content of the reponse.
     assert len(responses) == 1
     assert query_id in responses[0].text
+
+  def test_prometheus_metrics(self):
+    """Test to check prometheus metrics"""
+    resp = self.get_and_check_status(self.PROMETHEUS_METRICS_URL)
+    assert len(resp) == 3
+    # check if metric shows up
+    assert 'statestore_subscriber_heartbeat_interval_time_min' in resp[0].text


[impala] 02/02: IMPALA-8585: Add tests for partitioned ACID tables

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 396d57709e147fd5ae9b692a225cc2174d59df6f
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Fri May 24 18:24:20 2019 +0200

    IMPALA-8585: Add tests for partitioned ACID tables
    
    Added e2e tests for partitioned ACID tables.
    
    Added some unit tests for file filtering with open and
    aborted write ids.
    
    Increased 'hive.compactor.worker.threads' to 4 to make compactions
    faster because they are terrible slow with only one thread.
    
    Change-Id: I6732db306459621a11f67a7263e9e06748fa35a8
    Reviewed-on: http://gerrit.cloudera.org:8080/13428
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../java/org/apache/impala/util/AcidUtils.java     |   8 ++
 .../java/org/apache/impala/util/AcidUtilsTest.java |  56 +++++++++
 fe/src/test/resources/hive-site.xml.py             |   2 +-
 .../queries/QueryTest/acid-partitioned.test        | 133 +++++++++++++++++++++
 tests/query_test/test_acid.py                      |   4 +
 5 files changed, 202 insertions(+), 1 deletion(-)

diff --git a/fe/src/main/java/org/apache/impala/util/AcidUtils.java b/fe/src/main/java/org/apache/impala/util/AcidUtils.java
index 3d294b1..5642533 100644
--- a/fe/src/main/java/org/apache/impala/util/AcidUtils.java
+++ b/fe/src/main/java/org/apache/impala/util/AcidUtils.java
@@ -51,11 +51,19 @@ public class AcidUtils {
   public static final String TABLE_IS_TRANSACTIONAL = "transactional";
   public static final String TABLE_TRANSACTIONAL_PROPERTIES = "transactional_properties";
 
+  // Regex pattern for files in base directories. The pattern matches strings like
+  // "base_0000005/abc.txt",
+  // "base_0000005/0000/abc.txt",
+  // "base_0000003_v0003217/000000_0"
   private static final Pattern BASE_PATTERN = Pattern.compile(
       "base_" +
       "(?<writeId>\\d+)" +
       "(?:_v(?<visibilityTxnId>\\d+))?" +
       "(?:/.*)?");
+
+  // Regex pattern for files in delta directories. The pattern matches strings like
+  // "delta_0000006_0000006/000000_0",
+  // "delta_0000009_0000009_0000/0000/def.txt"
   private static final Pattern DELTA_PATTERN = Pattern.compile(
       "delta_" +
        "(?<minWriteId>\\d+)_" +
diff --git a/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java b/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java
index 0019327..080039a 100644
--- a/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java
+++ b/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java
@@ -87,6 +87,7 @@ public class AcidUtilsTest {
           "abc/",
           "abc/base_0000006/", // Not at root, so shouldn't be handled.
           "base_00000100/def.txt"},
+      // <tableName>:<highWatermark>:<minOpenWriteId>:<openWriteIds>:<abortedWriteIds>
         "default.test:10:1234:1,2,3",
         new String[]{
           "base_0000005/abc.txt",
@@ -94,6 +95,61 @@ public class AcidUtilsTest {
   }
 
   @Test
+  public void testOpenTransactions() {
+    assertFiltering(new String[]{
+          "base_01.txt",
+          "post_upgrade.txt",
+          "base_0000005/",
+          "base_0000005/abc.txt",
+          "base_0000005/0000/",
+          "base_0000005/0000/abc.txt",
+          "delta_0000006_0000006_0000/",
+          "delta_0000006_0000006_0000/000000_0",
+          "delta_0000007_0000007_0000/",
+          "delta_0000007_0000007_0000/000000_0",
+          "delta_0000008_0000008_0000/",
+          "delta_0000008_0000008_0000/000000_0",
+          "delta_0000009_0000009_0000/",
+          "delta_0000009_0000009_0000/000000_0",
+          "delta_0000009_0000009_0000/0000/def.txt"},
+        "default.test:10:6:6,7,8:", // 6,7,8 are open write ids
+        new String[]{
+          "base_01.txt",
+          "post_upgrade.txt",
+          "base_0000005/abc.txt",
+          "base_0000005/0000/abc.txt",
+          "delta_0000009_0000009_0000/000000_0",
+          "delta_0000009_0000009_0000/0000/def.txt"});
+  }
+
+  @Test
+  public void testAbortedTransactions() {
+    assertFiltering(new String[]{
+          "base_01.txt",
+          "post_upgrade.txt",
+          "base_0000005/",
+          "base_0000005/abc.txt",
+          "base_0000005/0000/",
+          "base_0000005/0000/abc.txt",
+          "delta_0000006_0000006_0000/",
+          "delta_0000006_0000006_0000/000000_0",
+          "delta_0000007_0000007_0000/",
+          "delta_0000007_0000007_0000/000000_0",
+          "delta_0000008_0000008_0000/",
+          "delta_0000008_0000008_0000/000000_0",
+          "delta_0000009_0000009_0000/",
+          "delta_0000009_0000009_0000/000000_0",
+          "delta_0000009_0000009_0000/0000/def.txt"},
+        "default.test:10:1337::7,8,9", // 7,8,9 are aborted write ids
+        new String[]{
+          "base_01.txt",
+          "post_upgrade.txt",
+          "base_0000005/abc.txt",
+          "base_0000005/0000/abc.txt",
+          "delta_0000006_0000006_0000/000000_0"});
+  }
+
+  @Test
   public void testPostCompactionBase() {
     assertFiltering(new String[]{
           "base_0000003_v0003217/",
diff --git a/fe/src/test/resources/hive-site.xml.py b/fe/src/test/resources/hive-site.xml.py
index 24a00b6..3e752c8 100644
--- a/fe/src/test/resources/hive-site.xml.py
+++ b/fe/src/test/resources/hive-site.xml.py
@@ -109,7 +109,7 @@ if hive_major_version >= 3:
 
    # Enable compaction workers. The compaction initiator is off by default
    # but configuring a worker thread allows manual compaction.
-   'hive.compactor.worker.threads': 1
+   'hive.compactor.worker.threads': 4
   })
 else:
   CONFIG.update({
diff --git a/testdata/workloads/functional-query/queries/QueryTest/acid-partitioned.test b/testdata/workloads/functional-query/queries/QueryTest/acid-partitioned.test
new file mode 100644
index 0000000..aae1972
--- /dev/null
+++ b/testdata/workloads/functional-query/queries/QueryTest/acid-partitioned.test
@@ -0,0 +1,133 @@
+====
+---- HIVE_QUERY
+# Create partitioned ACID table in Hive and query it in Impala.
+use $DATABASE;
+create table pt (i int)
+partitioned by (p int) tblproperties (
+  'transactional'='true',
+  'transactional_properties'='insert_only');
+insert into pt partition (p=1) values (10), (11);
+insert into pt partition (p=2) values (20);
+====
+---- QUERY
+invalidate metadata pt;
+select p, i from pt order by i;
+---- RESULTS
+1,10
+1,11
+2,20
+---- TYPES
+INT,INT
+====
+---- HIVE_QUERY
+use $DATABASE;
+insert into pt partition (p=2) values (21);
+====
+---- QUERY
+refresh pt;
+select p, i from pt order by i;
+---- RESULTS
+1,10
+1,11
+2,20
+2,21
+---- TYPES
+INT,INT
+====
+---- HIVE_QUERY
+use $DATABASE;
+insert overwrite table pt partition (p=1) values (12);
+insert into pt partition (p=2) values (22);
+====
+---- QUERY
+refresh pt;
+select p, i from pt order by i;
+---- RESULTS
+1,12
+2,20
+2,21
+2,22
+---- TYPES
+INT,INT
+====
+---- HIVE_QUERY
+use $DATABASE;
+alter table pt partition(p=2) compact 'major' and wait;
+====
+---- QUERY
+refresh pt;
+select p, i from pt order by i;
+---- RESULTS
+1,12
+2,20
+2,21
+2,22
+---- TYPES
+INT,INT
+====
+---- HIVE_QUERY
+# Create partitioned ACID table and use dynamic partitioning during insert.
+use $DATABASE;
+create table pt_dyn (i int)
+partitioned by (sp int, dp int) tblproperties (
+  'transactional'='true',
+  'transactional_properties'='insert_only');
+insert into table pt_dyn partition(sp=1, dp) select * from pt;
+insert into table pt_dyn partition(sp=3, dp) select 30, 3;
+====
+---- QUERY
+invalidate metadata pt_dyn;
+select sp, dp, i from pt_dyn order by i;
+---- RESULTS
+1,1,12
+1,2,20
+1,2,21
+1,2,22
+3,3,30
+---- TYPES
+INT,INT,INT
+====
+---- QUERY
+# Create non-ACID partitioned table in Impala and upgrade it to a
+# transactional one in Hive.
+create table upgraded_pt (i int) partitioned by (p int);
+insert into upgraded_pt partition (p=1) values (10);
+insert into upgraded_pt partition (p=2) values (20), (21);
+====
+---- HIVE_QUERY
+use $DATABASE;
+alter table upgraded_pt set tblproperties (
+  'transactional' = 'true',
+  'transactional_properties' = 'insert_only');
+insert into upgraded_pt partition(p=1) values (11);
+insert into upgraded_pt partition (p=2) values (22);
+====
+---- QUERY
+refresh upgraded_pt;
+select p, i from upgraded_pt order by i;
+---- RESULTS
+1,10
+1,11
+2,20
+2,21
+2,22
+---- TYPES
+INT,INT
+====
+---- HIVE_QUERY
+use $DATABASE;
+alter table upgraded_pt partition(p=1) compact 'major' and wait;
+alter table upgraded_pt partition(p=2) compact 'major' and wait;
+====
+---- QUERY
+refresh upgraded_pt;
+select p, i from upgraded_pt order by i;
+---- RESULTS
+1,10
+1,11
+2,20
+2,21
+2,22
+---- TYPES
+INT,INT
+====
diff --git a/tests/query_test/test_acid.py b/tests/query_test/test_acid.py
index bd40a41..74adc5d 100644
--- a/tests/query_test/test_acid.py
+++ b/tests/query_test/test_acid.py
@@ -47,6 +47,10 @@ class TestAcid(ImpalaTestSuite):
   def test_acid_negative(self, vector, unique_database):
     self.run_test_case('QueryTest/acid-negative', vector, use_db=unique_database)
 
+  @SkipIfHive2.acid
+  def test_acid_partitioned(self, vector, unique_database):
+    self.run_test_case('QueryTest/acid-partitioned', vector, use_db=unique_database)
+
 # TODO(todd): further tests to write:
 #  TRUNCATE, once HIVE-20137 is implemented.
 #  INSERT OVERWRITE with empty result set, once HIVE-21750 is fixed.