-
Notifications
You must be signed in to change notification settings - Fork 23
Add MemQ support for FlinkSQL 1.18 #135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package com.pinterest.flink.connector.psc.source.metrics; | ||
|
|
||
| import com.pinterest.psc.common.TopicUriPartition; | ||
| import com.pinterest.psc.metrics.Metric; | ||
| import com.pinterest.psc.metrics.MetricName; | ||
|
|
||
| import java.util.Map; | ||
| import java.util.function.Predicate; | ||
|
|
||
| class MemqSourceReaderMetricsUtil { | ||
|
|
||
| public static final String MEMQ_CONSUMER_METRIC_GROUP = "memq-consumer-metrics"; | ||
| public static final String BYTES_CONSUMED_TOTAL = "bytes.consumed.total"; | ||
| public static final String NOTIFICATION_RECORDS_LAG_MAX = "notification.records.lag.max"; | ||
|
|
||
| protected static Predicate<Map.Entry<MetricName, ? extends Metric>> createBytesConsumedFilter() { | ||
| return entry -> | ||
| entry.getKey().group().equals(MEMQ_CONSUMER_METRIC_GROUP) | ||
| && entry.getKey().name().equals(BYTES_CONSUMED_TOTAL); | ||
| } | ||
|
|
||
| protected static Predicate<Map.Entry<MetricName, ? extends Metric>> createRecordsLagFilter(TopicUriPartition tp) { | ||
| return entry -> | ||
| entry.getKey().group().equals(MEMQ_CONSUMER_METRIC_GROUP) | ||
| && entry.getKey().name().equals(NOTIFICATION_RECORDS_LAG_MAX); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |
| import com.pinterest.psc.exception.ClientException; | ||
| import com.pinterest.psc.metrics.Metric; | ||
| import com.pinterest.psc.metrics.MetricName; | ||
| import java.util.Iterator; | ||
| import org.apache.flink.annotation.PublicEvolving; | ||
| import org.apache.flink.metrics.Counter; | ||
| import org.apache.flink.metrics.MetricGroup; | ||
|
|
@@ -153,6 +154,19 @@ public void recordCurrentOffset(TopicUriPartition tp, long offset) { | |
| offsets.get(tp).currentOffset = offset; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the {@link Offset} tracker for the given partition, allowing callers to | ||
| * cache it and update offsets directly without repeated HashMap lookups. | ||
| * | ||
| * @param tp the topic partition to get the tracker for | ||
| * @return the Offset tracker | ||
| * @throws IllegalArgumentException if the partition is not tracked | ||
| */ | ||
| public Offset getOffsetTracker(TopicUriPartition tp) { | ||
| checkTopicPartitionTracked(tp); | ||
| return offsets.get(tp); | ||
| } | ||
|
|
||
| /** | ||
| * Update the latest committed offset of the given {@link TopicUriPartition}. | ||
| * | ||
|
|
@@ -180,8 +194,21 @@ public void recordFailedCommit() { | |
| * @param consumer Kafka consumer | ||
| */ | ||
| public void registerNumBytesIn(PscConsumer<?, ?> consumer) throws ClientException { | ||
| Predicate<Map.Entry<MetricName, ? extends Metric>> filter = | ||
| KafkaSourceReaderMetricsUtil.createBytesConsumedFilter(); | ||
| String backendType = getBackendFromTags(consumer.metrics()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to handle potential
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in |
||
| Predicate<Map.Entry<MetricName, ? extends Metric>> filter; | ||
| switch (backendType) { | ||
| case PscUtils.BACKEND_TYPE_KAFKA: | ||
| filter = KafkaSourceReaderMetricsUtil.createBytesConsumedFilter(); | ||
| break; | ||
| case PscUtils.BACKEND_TYPE_MEMQ: | ||
| filter = MemqSourceReaderMetricsUtil.createBytesConsumedFilter(); | ||
| break; | ||
| default: | ||
| LOG.warn( | ||
| "Unsupported backend type: \"{}\". Metric \"{}\" may not be reported correctly.", | ||
| backendType, MetricNames.IO_NUM_BYTES_IN); | ||
| return; | ||
| } | ||
| this.bytesConsumedTotalMetric = MetricUtil.getPscMetric(consumer.metrics(), filter); | ||
| } | ||
|
|
||
|
|
@@ -288,25 +315,35 @@ private void checkTopicPartitionTracked(TopicUriPartition tp) { | |
| case PscUtils.BACKEND_TYPE_KAFKA: | ||
| filter = KafkaSourceReaderMetricsUtil.createRecordLagFilter(tp); | ||
| break; | ||
| case PscUtils.BACKEND_TYPE_MEMQ: | ||
| filter = MemqSourceReaderMetricsUtil.createRecordsLagFilter(tp); | ||
| break; | ||
| default: | ||
| LOG.warn( | ||
| String.format( | ||
| "Unsupported backend type \"%s\". " | ||
| + "Metric \"%s\" may not be reported correctly. ", | ||
| backendType, MetricNames.PENDING_RECORDS)); | ||
| "Unsupported backend type \"{}\". Metric \"{}\" may not be reported correctly.", | ||
| backendType, MetricNames.PENDING_RECORDS); | ||
| return null; | ||
| } | ||
| return MetricUtil.getPscMetric(metrics, filter); | ||
| try { | ||
| return MetricUtil.getPscMetric(metrics, filter); | ||
| } catch (IllegalStateException e) { | ||
| LOG.debug("Metric not yet available for backend \"{}\", will retry on next poll cycle.", backendType); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| private static String getBackendFromTags(Map<MetricName, ? extends Metric> metrics) { | ||
| // sample the first entry to get the backend type | ||
| return metrics.keySet().iterator().next().tags().get("backend"); | ||
| Iterator<MetricName> it = metrics.keySet().iterator(); | ||
| if (!it.hasNext()) { | ||
| return "unknown"; | ||
| } | ||
| String backend = it.next().tags().get("backend"); | ||
| return backend != null ? backend : "unknown"; | ||
| } | ||
|
|
||
| private static class Offset { | ||
| long currentOffset; | ||
| long committedOffset; | ||
| public static class Offset { | ||
| public long currentOffset; | ||
| public long committedOffset; | ||
|
Comment on lines
+344
to
+346
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the reason for making this part of the public API? Any potential harm?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it needs to be accessed from multiple sub packages. I don't think there is potential harm - it's just a data holder for metrics tracking and doesn't impact any business logic. |
||
|
|
||
| Offset(long currentOffset, long committedOffset) { | ||
| this.currentOffset = currentOffset; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| package com.pinterest.psc.config; | ||
|
|
||
| import com.pinterest.memq.client.commons.ConsumerConfigs; | ||
| import com.pinterest.psc.common.TopicUri; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.Properties; | ||
|
|
||
| public class PscMetadataClientToMemqConsumerConfigConverter extends PscMetadataClientToBackendMetatadataClientConfigCoverter { | ||
| @Override | ||
| protected Map<String, String> getConfigConverterMap() { | ||
| return new HashMap<String, String>() { | ||
| private static final long serialVersionUID = 1L; | ||
|
|
||
| { | ||
| put(PscConfiguration.PSC_METADATA_CLIENT_ID, ConsumerConfigs.CLIENT_ID); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| @Override | ||
| public Properties convert(PscConfigurationInternal pscConfigurationInternal, TopicUri topicUri) { | ||
| return super.convert(pscConfigurationInternal, topicUri); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To confirm, is
tpa no-op for this function?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's also a no-op on Kafka side. for some reason when we ported the Kafka connector implementation to PSC, filtering further by
tpreturned empty results, and it only returned metrics when we didn't filter further bytp. We left the method signature untouched but commented out thetpfiltering logic.