From 1894b2c9ab8c007f38be77a4cce829f59c95c69c Mon Sep 17 00:00:00 2001 From: Brennan Vincent Date: Fri, 2 Apr 2021 14:53:21 -0400 Subject: [PATCH 1/4] Turn off stats by default (hopefully temporarily) --- src/sql/src/kafka_util.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sql/src/kafka_util.rs b/src/sql/src/kafka_util.rs index 6953f519e6261..12774a8f71188 100644 --- a/src/sql/src/kafka_util.rs +++ b/src/sql/src/kafka_util.rs @@ -148,10 +148,11 @@ pub fn extract_config( // The range of values comes from `statistics.interval.ms` in // https://github.com/edenhill/librdkafka/blob/master/CONFIGURATION.md ValType::Number(0, 86_400_000), - ) - .set_default(Some( - chrono::Duration::seconds(1).num_milliseconds().to_string(), - )), + ), + // TODO - stats can be reenabled by default when we figure out why it's leaking memory. + // .set_default(Some( + // chrono::Duration::seconds(1).num_milliseconds().to_string(), + // )), Config::new( "topic_metadata_refresh_interval_ms", // The range of values comes from `topic.metadata.refresh.interval.ms` in From c5a9128b5d9d46139bf49f258e5e44b7aa64b9b2 Mon Sep 17 00:00:00 2001 From: Chris Golden Date: Fri, 2 Apr 2021 12:22:30 -0700 Subject: [PATCH 2/4] Fixup tests that rely on kafka statistics --- test/catalog-compat/catcompatck/catcompatck | 9 ++-- test/testdrive/avro-sources.td | 56 +++++++++++---------- test/testdrive/kafka-stats.td | 1 + 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/test/catalog-compat/catcompatck/catcompatck b/test/catalog-compat/catcompatck/catcompatck index 512a1941a11ee..660291da6ab89 100755 --- a/test/catalog-compat/catcompatck/catcompatck +++ b/test/catalog-compat/catcompatck/catcompatck @@ -201,11 +201,12 @@ a b 3 1 1 2 +# TODO: Uncomment out test once kafka stats are enabled by default once again # Kafka metrics for the real_time_src should be enabled now # Count should be 2 because there are two materialized views on real_time_src # If real_time_src_no_stats were also emitting stats, there would be 3 rows -> SELECT count(*) FROM mz_kafka_consumer_statistics; -count ------ -2 +# > SELECT count(*) FROM mz_kafka_consumer_statistics; +# count +# ----- +# 2 EOF diff --git a/test/testdrive/avro-sources.td b/test/testdrive/avro-sources.td index 1e75ad4a8362c..a8a8aa88dbf90 100644 --- a/test/testdrive/avro-sources.td +++ b/test/testdrive/avro-sources.td @@ -333,20 +333,21 @@ a b 5 6 9 10 +# TODO: Uncomment out test once kafka stats are enabled by default once again # There should be two partitions for the last created source / consumer (non_dbz_data_varying_partition_2) -> SELECT count(*) FROM mz_kafka_consumer_statistics GROUP BY consumer_name; -count ------ -1 -1 -1 -1 -2 -2 -2 -2 -2 -2 +# > SELECT count(*) FROM mz_kafka_consumer_statistics GROUP BY consumer_name; +# count +# ----- +# 1 +# 1 +# 1 +# 1 +# 2 +# 2 +# 2 +# 2 +# 2 +# 2 > CREATE MATERIALIZED SOURCE non_dbz_data_varying_partition_3 FROM KAFKA BROKER '${testdrive.kafka-addr}' TOPIC 'testdrive-non-dbz-data-varying-partition-${testdrive.seed}' @@ -367,21 +368,22 @@ a b 9 10 11 12 +# TODO: Uncomment out test once kafka stats are enabled by default once again # There should three partitions for the last three sources / consumers (non_dbz_data_varying_partition_[123]) -> SELECT count(*) FROM mz_kafka_consumer_statistics GROUP BY consumer_name; -count ------ -1 -1 -1 -1 -2 -2 -2 -2 -3 -3 -3 +# > SELECT count(*) FROM mz_kafka_consumer_statistics GROUP BY consumer_name; +# count +# ----- +# 1 +# 1 +# 1 +# 1 +# 2 +# 2 +# 2 +# 2 +# 3 +# 3 +# 3 $ set-sql-timeout duration=12.7s diff --git a/test/testdrive/kafka-stats.td b/test/testdrive/kafka-stats.td index 82c56c9da5606..062bababfe956 100644 --- a/test/testdrive/kafka-stats.td +++ b/test/testdrive/kafka-stats.td @@ -22,6 +22,7 @@ $ kafka-create-topic topic=data > CREATE SOURCE data FROM KAFKA BROKER '${testdrive.kafka-addr}' TOPIC 'testdrive-data-${testdrive.seed}' + WITH (statistics_interval_ms = 0) FORMAT AVRO USING SCHEMA '${schema}' > CREATE MATERIALIZED VIEW test1 AS From 71678c60da84fa61a18dc1fc4065d6f7a3f1f256 Mon Sep 17 00:00:00 2001 From: Chris Golden Date: Fri, 2 Apr 2021 12:43:24 -0700 Subject: [PATCH 3/4] Actually enable stats in this test file --- test/testdrive/kafka-stats.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/testdrive/kafka-stats.td b/test/testdrive/kafka-stats.td index 062bababfe956..c46cf43213e73 100644 --- a/test/testdrive/kafka-stats.td +++ b/test/testdrive/kafka-stats.td @@ -22,7 +22,7 @@ $ kafka-create-topic topic=data > CREATE SOURCE data FROM KAFKA BROKER '${testdrive.kafka-addr}' TOPIC 'testdrive-data-${testdrive.seed}' - WITH (statistics_interval_ms = 0) + WITH (statistics_interval_ms = 1000) FORMAT AVRO USING SCHEMA '${schema}' > CREATE MATERIALIZED VIEW test1 AS From c9576dcf27ea05be700d00a70055173ad3a2c420 Mon Sep 17 00:00:00 2001 From: Chris Golden Date: Fri, 2 Apr 2021 12:45:34 -0700 Subject: [PATCH 4/4] Comment out unused function --- src/sql/src/kafka_util.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sql/src/kafka_util.rs b/src/sql/src/kafka_util.rs index 12774a8f71188..e6a730877fa28 100644 --- a/src/sql/src/kafka_util.rs +++ b/src/sql/src/kafka_util.rs @@ -72,10 +72,10 @@ impl Config { } // Allows for returning a default value for this configuration option - fn set_default(mut self, d: Option) -> Self { - self.default = d; - self - } + // fn set_default(mut self, d: Option) -> Self { + // self.default = d; + // self + // } // Get the appropriate String to use as the Kafka config key. fn get_key(&self) -> String {