You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/12/13 06:20:48 UTC
[GitHub] [pinot] navina opened a new pull request, #9977: Emit consumer partition lag from the server
navina opened a new pull request, #9977:
URL: https://github.com/apache/pinot/pull/9977
This PR introduces a way to produce partition lag metrics for the supported low level realtime consumers. Major changes include:
* Changed signature of `RealtimeSegmentDataManager#getConsumerPartitionState` to include a boolean `skipOffsetLag`. **Reasoning**:
- `fetchUpstreamLatest` involves making a call to the datasource. If invoked frequently, it can overwhelm the data source broker.
- Debug APIs should still be able to fetch the latest upstream offset. This is useful during debugging.
* New method in `RealtimeSegmentDataManager#handleConsumptionStatus` - This is used to signal the segment data manager from external components like status checker". For now, it will be used to signal whether the realtime consumer segment has "caught up" per its consumption based status checker.
**Labels**: `observability` , `feature`, `release-notes`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jugomezv commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
jugomezv commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1050886269
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
We had experimented with similar approach about a month back internally and decided against having a lag metric for partition and come up instead with [this](https://github.com/apache/pinot/pull/9994) approach. Among the motivation we have were included the explosion of metrics if we do one metric for partition. We also address issues like aging of time lag measures and handling of events that affect partition hosting. These extra items do make sense for time based but not for offset based so I do think we should keep offset and time based lag separate. May be focus this pr on offset and the other one in lag. I could modify my PR to also report the partition creating the largest lag if that is something you think it will be useful. We should also consider whether we want to combine the offset metric on a single one: I think we can reuse the tracker object I created on my PR and disable aging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1048973880
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
But it does not show that. Each time the consumer does not poll the stream, it is that much farther away from freshness. Unless the stream is idle, but then we are not too worried about that right? The idea is to know how stale the data is when queries are being served in a busy production environment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1350027454
> @mcvsubbu is this the work being done by linkedin - https://github.com/jugomezv/pinot/pull/3/files ?
yes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1350004868
@mcvsubbu is this the work being done by linkedin - https://github.com/jugomezv/pinot/pull/3/files ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1047905553
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -617,6 +624,40 @@ private boolean processStreamEvents(MessageBatch messagesAndOffsets, long idlePi
return prematureExit;
}
+ private void updateConsumerLagMetrics() {
+ boolean emitRecordsLag = true;
+ boolean emitAvailabilityLag = true;
+ if (_caughtUpWithUpstream) {
Review Comment:
It is ok for the metric to spike in the short term. But we need to know how far behind we are, and how fast we are catching up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1047906528
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
Does this measure time lag? What is availability lag?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jugomezv commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
jugomezv commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1050924503
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
@navina Navina, I sent an email invite for Monday to see if we can converge on these PRs, please forward as required on your side. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1050244666
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -65,8 +65,21 @@ protected static PinotDataBufferMemoryManager getMemoryManager(String consumerDi
* @return Per-partition consumer's status, which typically includes last consumed message timestamp,
* latest available upstream offset etc
*/
- public abstract Map<String, ConsumerPartitionState> getConsumerPartitionState();
+ public abstract Map<String, ConsumerPartitionState> getConsumerPartitionState(boolean skipOffsetLag);
Review Comment:
Is this change related? I feel this is more like a performance optimization to avoid always fetching latest offset?
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/HLRealtimeSegmentDataManager.java:
##########
@@ -437,6 +442,11 @@ public Map<String, PartitionLagState> getPartitionToLagState(
throw new UnsupportedOperationException();
}
+ @Override
+ public void handleConsumptionStatus(boolean caughtUp) {
Review Comment:
Suggest renaming it to `notifyConsumptionCaughtUp()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1048973880
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
But it does not show that. Each minute the consumer does not poll the stream, it is that much farther away from freshness. Unless the stream is idle, but then we are not too worried about that right? The idea is to know how stale the data is when queries are being served in a busy production environment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1382280204
Closing this as the work has been included in #9994 .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1349986645
Yes, Basically, the approach is to have a time-based lag as the diff between `System.getCurrentMillis()` and `msgMetadata.getRecordIngestionTimeMs()` and emit that as a metric.
However, we are adding more logic around this to keep increasing the time when we the consumer is busy building segments (for example). Or, when when there is no consuming partition for a partition.
@jugomezv will post a PR soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1049012304
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
>show how far behind our consumer is from the input stream irrespective of what the engine is doing
my bad. I corrected my typo above. It is how far our consumer is with respect to the last processed message.
> The idea is to know how stale the data is when queries are being served in a busy production environment.
The idea is simply to track the progress the consumer is making regardless of what the engine is doing. So, yes, it will increase when we are not polling the stream. that is expected and that is what we want to know.
> Does this take care of the lag when a server is building segments or in HOLD state awaiting segment completion?
Can you explain what do you mean by this? availability lag should increase when the server is not polling and is busy with segment building/committing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1046735518
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -617,6 +624,40 @@ private boolean processStreamEvents(MessageBatch messagesAndOffsets, long idlePi
return prematureExit;
}
+ private void updateConsumerLagMetrics() {
+ boolean emitRecordsLag = true;
+ boolean emitAvailabilityLag = true;
+ if (_caughtUpWithUpstream) {
Review Comment:
why has this condition? we should always publish the metric right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1048060753
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
Yes. `availabilityLag = now() - recordMetadata.timestamp`
It emits this metrics at a per-partition level.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] gortiz commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
gortiz commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1350666677
> This PR introduces a way to produce partition lag metrics for the supported low level realtime consumers.
Don't we already have that metric exposes by kafka consumer library?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] gortiz commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
gortiz commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1357228195
I'm not against the feature but as said, I don't understand why we need it. Kafka already exports the replication lag for each consumer. They are exported in JMX right now and the only thing we need to read it is to add it to the prometheus yml file.
I've been talking with Navina and it seems that consumers export metrics classifying them by consumer.id, which right now is a random value, but we could change it to be the name of the server plus the table, so we can classify that.
It may be the case that we need to add some extra metrics but:
1. We should study whether that new metric can be useful to third parties, in such case it may be introduced in Kafka itself
2. I don't think we should make PinotMetricRegistry plugable, as I explained [here](https://github.com/apache/pinot/issues/8762)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1050954430
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
@jugomezv :
> explosion of metrics if we do one metric for partition.
can you clarify why adding this partition-level metric would cause metric explosion when the realtime engine already has a bunch of per-partition-level metric (eg. `HIGHEST_STREAM_OFFSET_CONSUMED` , `LLC_PARTITION_CONSUMING`, `UPSERT_PRIMARY_KEYS_COUNT` etc )?
> These extra items do make sense for time based but not for offset based so I do think we should keep offset and time based lag separate. May be focus this pr on offset and the other one in lag.
This PR does not do offset-based lag as it is pretty expensive. The initial version of the PR did include the offset lag. but there was a flag that made sure it doesn't actually emit the metric. I have updated the PR since to only publish time-based lag.
> We also address issues like aging of time lag measures and handling of events that affect partition hosting.
By "aging of time lag measure", do you mean incrementing the lag when we are not polling for messages? That would only make the lag trend-up when: 1. there is no data in the input stream or 2. consumption rate is lower than the ingestion rate . From a monitoring stand-point, it is hard to define a rule that would not result in false positives.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1050939027
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeSegmentDataManager.java:
##########
@@ -65,8 +65,21 @@ protected static PinotDataBufferMemoryManager getMemoryManager(String consumerDi
* @return Per-partition consumer's status, which typically includes last consumed message timestamp,
* latest available upstream offset etc
*/
- public abstract Map<String, ConsumerPartitionState> getConsumerPartitionState();
+ public abstract Map<String, ConsumerPartitionState> getConsumerPartitionState(boolean skipOffsetLag);
Review Comment:
It is related. We don't want to fetch latest offset every time we emit metrics. This would put unusual load on the source broker since we will make #replica calls for each topic partition individually.
I would think of it more like safety - Pinot should not kill the source broker. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jugomezv commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
jugomezv commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1051009453
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
Our argument to not have a per partition metric is that it just clouds the user view, what we care about in most cases is the worst case scenario which is what we are handling. Our SREs complained about the per metrica approach as it would make their job harder, they wanted a single point metric that tells them the worst case scenario.
Ack on this having removed the support for offset, but I think that was important.
As for aging, imagine you collect one sample that says event delay last measure was 100ms and then we do get stuck committing a segment, that may take several seconds so the best estimate of consumption delay is the age of the last worst sample plus the sample itself. If we keep posting the sample we are giving the wrong idea to the customer. When we do try to consume and no events are found we display 0 delay as if there were events we will consume them with no delay. Please let me know if the meeting I set for next week suits your schedule and if not let me know what time will be best.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1349689401
We will be putting up a PR very soon on time based lag. Could we wait to take a look and see if we can come up with something that addresses both requirements?
thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1047540796
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -255,6 +255,11 @@ public void deleteSegmentFile() {
private StreamPartitionMsgOffset _finalOffset; // Used when we want to catch up to this one
private volatile boolean _shouldStop = false;
+ /** This variable will be set by the configured {@literal IngestionBasedConsumptionStatusChecker} when the segment is
+ * caughtup.
+ */
+ private volatile boolean _caughtUpWithUpstream = false;
Review Comment:
Very good questions and I think we will encounter can increase in the lag during traffic spike. However, we will be monitoring on the "trend" of the lag, where it should always be trending down or remain stable.
The problem with pause/resume is that the segment or table data manager doesn't know if the consumer has been paused because it is simply treated as a force commit. I think we still want to have visibility when the stream is paused. But the other metric `LLC_PARTITION_CONSUMING` will not be true in this case. So, the monitoring rule should be something like:
ALERT IF (`LLC_PARTITION_CONSUMING` == 1 && `availabilityLagMs` is trending up ) for a period of 15min
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1049014139
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
Does the metric show the increase, is the question. Correct me if I am wrong, but the metric will only show the lag when the partition last ingested events. When segment build starts, the metric should increase in lag, and I don't think that is being handed in the code that you have.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1051081259
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
> Our SREs complained about the per metrics approach as it would make their job harder, they wanted a single point metric that tells them the worst case scenario.
this is a good point. But we and some users have often found the need to look at per-partition level metrics during debugging. This is esp. true for non-kafka sources for which pinot provides no equivalent lag metrics.
> Ack on this having removed the support for offset, but I think that was important.
In most practical scenarios, offset difference doesn't tell you much. But it can be useful during debugging and this is definitely one of the widely asked/popular metric. However, computing this in Pinot layer is quite expensive as it requires sending a request to the source kafka broker each time. Instead, the kafka consumer client provides APIs to fetch this metric that it holds internally. I think going forward, we should leverage that API or directly try to export the kafka consumer client's mbean which contains the lag.
Moreover, the offset lag doesn't apply to non-kafka sources anyway.
> When we do try to consume and no events are found we display 0 delay as if there were events we will consume them with no delay
That's not what this PR is doing though. When there are no events, the delay remains the same as the last sample. This along with the consumption rate should tell you whether there were any new events or not.
> Please let me know if the meeting I set for next week suits your schedule and if not let me know what time will be best.
I don't see an invite in any of my emails. I can DM you my email in the OSS slack.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1046737088
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -617,6 +624,40 @@ private boolean processStreamEvents(MessageBatch messagesAndOffsets, long idlePi
return prematureExit;
}
+ private void updateConsumerLagMetrics() {
+ boolean emitRecordsLag = true;
+ boolean emitAvailabilityLag = true;
+ if (_caughtUpWithUpstream) {
Review Comment:
This is to handle the catch up scenario. If the partition is still "catching up" to the upstream, this metric will spike, even though it is not indicative of any issue with consumption. Thus, we wanted to avoid the potential for false alerts with this metric.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1349993251
> Yes, Basically, the approach is to have a time-based lag as the diff between System.getCurrentMillis() and msgMetadata.getRecordIngestionTimeMs() and emit that as a metric.
That's pretty much what this PR is doing
> However, we are adding more logic around this to keep increasing the time when we the consumer is busy building segments (for example). Or, when when there is no consuming partition for a partition.
I am fairly certain we will re-visit this metric as we need to see its behavior and usefulness in the wild. So, we can do the improvements as a follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1350198523
> @mcvsubbu I see it as a different solution where you are sampling to compute the worst ingestion lag at a table-level and emitting a metric based on some more signals from the realtime engine. So, I think this PR and the other can co-exist.
The reason I was thinking of holding off is that some of the areas are common -- detecting partitions not served anymore, etc. Or, maybe we don't need that in the offset case ? If we do, then the equivalent will be to poll the stream periodically to see what the current highest offset is. I don't think you are doing that are you?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1350201055
>The reason I was thinking of holding off is that some of the areas are common -- detecting partitions not served anymore, etc. Or, maybe we don't need that in the offset case ? If we do, then the equivalent will be to poll the stream periodically to see what the current highest offset is. I don't think you are doing that are you?
Nope. that would kill the source. We are only dealing with time-based lag here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1047579926
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -255,6 +255,11 @@ public void deleteSegmentFile() {
private StreamPartitionMsgOffset _finalOffset; // Used when we want to catch up to this one
private volatile boolean _shouldStop = false;
+ /** This variable will be set by the configured {@literal IngestionBasedConsumptionStatusChecker} when the segment is
+ * caughtup.
+ */
+ private volatile boolean _caughtUpWithUpstream = false;
Review Comment:
I just realized that pause/resume works at a table level, which means it is for all stream partitions. Whereas, this metric is created at a partition-level, rather than a stream level. So, I don't think we can even detect pause/resume in the table data manager. This will require yet another indicator on the controller side
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1046726884
##########
pinot-tools/src/main/resources/examples/stream/airlineStats/airlineStats_realtime_table_config.json:
##########
@@ -25,8 +25,7 @@
"stream.kafka.zk.broker.url": "localhost:2191/kafka",
"stream.kafka.broker.list": "localhost:19092",
"realtime.segment.flush.threshold.time": "3600000",
- "realtime.segment.flush.threshold.size": "50000",
- "stream.kafka.consumer.prop.auto.offset.reset": "smallest"
+ "realtime.segment.flush.threshold.size": "50000"
Review Comment:
why remove this?
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -255,6 +255,11 @@ public void deleteSegmentFile() {
private StreamPartitionMsgOffset _finalOffset; // Used when we want to catch up to this one
private volatile boolean _shouldStop = false;
+ /** This variable will be set by the configured {@literal IngestionBasedConsumptionStatusChecker} when the segment is
+ * caughtup.
+ */
+ private volatile boolean _caughtUpWithUpstream = false;
Review Comment:
This will be true once the server is caughtup, how do you handle the scenario that a sudden traffic burst or consumer pause/resume?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1350063920
@mcvsubbu I see it as a different solution where you are sampling to compute the worst ingestion lag at a table-level and emitting a metric based on some more signals from the realtime engine. So, I think this PR and the other can co-exist.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1047533146
##########
pinot-tools/src/main/resources/examples/stream/airlineStats/airlineStats_realtime_table_config.json:
##########
@@ -25,8 +25,7 @@
"stream.kafka.zk.broker.url": "localhost:2191/kafka",
"stream.kafka.broker.list": "localhost:19092",
"realtime.segment.flush.threshold.time": "3600000",
- "realtime.segment.flush.threshold.size": "50000",
- "stream.kafka.consumer.prop.auto.offset.reset": "smallest"
+ "realtime.segment.flush.threshold.size": "50000"
Review Comment:
I thought I had modified this for testing. looks like I accidentally deleted the offset reset config too. tks for catching this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] PrachiKhobragade commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
PrachiKhobragade commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1050185091
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -617,6 +624,28 @@ private boolean processStreamEvents(MessageBatch messagesAndOffsets, long idlePi
return prematureExit;
}
+ private void updateConsumerLagMetrics() {
+ boolean emitAvailabilityLag = true;
+ if (_caughtUpWithUpstream) {
+ Map<String, PartitionLagState> lagStateMap = getPartitionToLagState(getConsumerPartitionState(true));
+ for (Map.Entry<String, PartitionLagState> entry : lagStateMap.entrySet()) {
+ String partitionId = entry.getKey();
+ PartitionLagState lagState = entry.getValue();
+ if (emitAvailabilityLag && !PartitionLagState.NOT_CALCULATED.equals(lagState.getAvailabilityLagMs())) {
+ try {
+ _serverMetrics.setValueOfPartitionGauge(_tableNameWithType, Integer.parseInt(partitionId),
+ ServerGauge.LLC_AVAILABILITY_LAG_MS, Long.parseLong(entry.getValue().getAvailabilityLagMs()));
+ } catch (NumberFormatException nfe) {
+ // do not do anything
+ _segmentLogger.debug("Failed to parse the availability lag {} returned from the consumer for partition {}",
+ lagState.getRecordsLag(), partitionId);
+ emitAvailabilityLag = false;
Review Comment:
We can break the loop here, no need for additional checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jugomezv commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
jugomezv commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1353414295
Hi guys: I have published the version of our approach here:[ ](https://github.com/apache/pinot/pull/9994) I would gladly merge any functionality you may need is not there already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1048893987
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
No. We are not handling that. This is purely to show how far behind our consumer is from the input stream irrespective of what the engine is doing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina closed pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina closed pull request #9977: Emit consumer partition lag from the server
URL: https://github.com/apache/pinot/pull/9977
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1347805589
@Jackie-Jiang : Please review. Let me know if this approach is what you had in mind for the catch-up scenario. Meanwhile I will try to add some tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1347845938
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9977?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#9977](https://codecov.io/gh/apache/pinot/pull/9977?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3ee418c) into [master](https://codecov.io/gh/apache/pinot/commit/6a5f58b8e194bee7211bb9bdc97ad855486b296d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6a5f58b) will **increase** coverage by `0.63%`.
> The diff coverage is `29.16%`.
```diff
@@ Coverage Diff @@
## master #9977 +/- ##
============================================
+ Coverage 24.49% 25.12% +0.63%
+ Complexity 53 44 -9
============================================
Files 1976 1976
Lines 106764 106808 +44
Branches 16243 16248 +5
============================================
+ Hits 26150 26837 +687
+ Misses 77929 77239 -690
- Partials 2685 2732 +47
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `25.12% <29.16%> (?)` | |
| integration2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9977?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...manager/realtime/HLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvSExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `0.00% <0.00%> (-80.28%)` | :arrow_down: |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
| [...ache/pinot/server/api/resources/DebugResource.java](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9EZWJ1Z1Jlc291cmNlLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/server/api/resources/TablesResource.java](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZXNSZXNvdXJjZS5qYXZh) | `29.78% <0.00%> (-9.37%)` | :arrow_down: |
| [.../helix/IngestionBasedConsumptionStatusChecker.java](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9Jbmdlc3Rpb25CYXNlZENvbnN1bXB0aW9uU3RhdHVzQ2hlY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `57.72% <20.58%> (+0.36%)` | :arrow_up: |
| [...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==) | `77.19% <100.00%> (+2.34%)` | :arrow_up: |
| [...a/org/apache/pinot/common/metrics/ServerGauge.java](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9TZXJ2ZXJHYXVnZS5qYXZh) | `96.55% <100.00%> (+0.25%)` | :arrow_up: |
| [.../apache/pinot/server/access/RequesterIdentity.java](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL2FjY2Vzcy9SZXF1ZXN0ZXJJZGVudGl0eS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/core/data/manager/realtime/TimerService.java](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvVGltZXJTZXJ2aWNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [378 more](https://codecov.io/gh/apache/pinot/pull/9977/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1352260070
> Don't we already have that metric exposes by kafka consumer library?
Yes. This PR is sort of a proxy until we add support for plugins to report metrics. I DM'd you more details. Let me know if you want another discussion offline. tks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on PR #9977:
URL: https://github.com/apache/pinot/pull/9977#issuecomment-1349856511
> We will be putting up a PR very soon on time based lag. Could we wait to take a look and see if we can come up with something that addresses both requirements? thanks.
Can you or whoever is working on this time-based lag share more details on your approach? It will be better if we can collaborate right away and get this working as we want this in the upcoming release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1047905063
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -255,6 +255,11 @@ public void deleteSegmentFile() {
private StreamPartitionMsgOffset _finalOffset; // Used when we want to catch up to this one
private volatile boolean _shouldStop = false;
+ /** This variable will be set by the configured {@literal IngestionBasedConsumptionStatusChecker} when the segment is
+ * caughtup.
+ */
+ private volatile boolean _caughtUpWithUpstream = false;
Review Comment:
idealstate has indication that pause is on
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1048850441
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
Does this take care of the lag when a server is building segments or in HOLD state awaiting segment completion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] navina commented on a diff in pull request #9977: Emit consumer partition lag from the server
Posted by GitBox <gi...@apache.org>.
navina commented on code in PR #9977:
URL: https://github.com/apache/pinot/pull/9977#discussion_r1048893987
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -45,7 +45,10 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
// Dedup metrics
DEDUP_PRIMARY_KEYS_COUNT("dedupPrimaryKeysCount", false),
CONSUMPTION_QUOTA_UTILIZATION("ratio", false),
- JVM_HEAP_USED_BYTES("bytes", true);
+ JVM_HEAP_USED_BYTES("bytes", true),
+ // Consumer lag metrics
+ LLC_RECORDS_LAG("recordCount", false),
+ LLC_AVAILABILITY_LAG_MS("availabilityLagMs", false);
Review Comment:
No. We are not handling that. This is purely to show how far behind our processing from input stream is, irrespective of what the engine is doing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org