-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Offset based realtime consumption status checker #7267
Changes from all commits
5618a77
c52ce09
0f6e9f4
5f4f533
4edfa3b
73f0f85
0975343
d208527
43546af
9f58f6c
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 |
---|---|---|
|
@@ -72,6 +72,7 @@ | |
import org.apache.pinot.spi.data.readers.GenericRow; | ||
import org.apache.pinot.spi.metrics.PinotMeter; | ||
import org.apache.pinot.spi.stream.MessageBatch; | ||
import org.apache.pinot.spi.stream.OffsetCriteria; | ||
import org.apache.pinot.spi.stream.PartitionGroupConsumer; | ||
import org.apache.pinot.spi.stream.PartitionGroupConsumptionStatus; | ||
import org.apache.pinot.spi.stream.PartitionLevelStreamConfig; | ||
|
@@ -287,6 +288,8 @@ public void deleteSegmentFile() { | |
private final boolean _nullHandlingEnabled; | ||
private final SegmentCommitterFactory _segmentCommitterFactory; | ||
|
||
private volatile StreamPartitionMsgOffset _latestStreamOffsetAtStartupTime = null; | ||
|
||
// TODO each time this method is called, we print reason for stop. Good to print only once. | ||
private boolean endCriteriaReached() { | ||
Preconditions.checkState(_state.shouldConsume(), "Incorrect state %s", _state); | ||
|
@@ -762,11 +765,14 @@ public long getLastConsumedTimestamp() { | |
return _lastLogTime; | ||
} | ||
|
||
@VisibleForTesting | ||
protected StreamPartitionMsgOffset getCurrentOffset() { | ||
public StreamPartitionMsgOffset getCurrentOffset() { | ||
return _currentOffset; | ||
} | ||
|
||
public StreamPartitionMsgOffset getLatestStreamOffsetAtStartupTime() { | ||
return _latestStreamOffsetAtStartupTime; | ||
} | ||
|
||
@VisibleForTesting | ||
protected SegmentBuildDescriptor getSegmentBuildDescriptor() { | ||
return _segmentBuildDescriptor; | ||
|
@@ -1363,6 +1369,16 @@ public LLRealtimeSegmentDataManager(SegmentZKMetadata segmentZKMetadata, TableCo | |
} | ||
_state = State.INITIAL_CONSUMING; | ||
|
||
// fetch latest stream offset | ||
try (StreamMetadataProvider metadataProvider = _streamConsumerFactory | ||
.createPartitionMetadataProvider(_clientId, _partitionGroupId)) { | ||
_latestStreamOffsetAtStartupTime = metadataProvider | ||
.fetchStreamPartitionOffset(OffsetCriteria.LARGEST_OFFSET_CRITERIA, /*maxWaitTimeMs*/5000); | ||
} catch (Exception e) { | ||
_segmentLogger.warn("Cannot fetch latest stream offset for clientId {} and partitionGroupId {}", _clientId, | ||
_partitionGroupId); | ||
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. would it be good to log the exception here? 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. I'm not sure. The only thing that can go wrong is timeout and if we log the exception, we'll clutter the logs with unhelpful stack trace of the timeout. If you still think it's better to log the exception, I can add it. |
||
} | ||
|
||
long now = now(); | ||
_consumeStartTime = now; | ||
long maxConsumeTimeMillis = _partitionLevelStreamConfig.getFlushThresholdTimeMillis(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
/** | ||
* Licensed to the Apache Software Foundation (ASF) under one | ||
* or more contributor license agreements. See the NOTICE file | ||
* distributed with this work for additional information | ||
* regarding copyright ownership. The ASF licenses this file | ||
* to you under the Apache License, Version 2.0 (the | ||
* "License"); you may not use this file except in compliance | ||
* with the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
package org.apache.pinot.server.starter.helix; | ||
|
||
import java.util.HashSet; | ||
import java.util.Set; | ||
import org.apache.pinot.common.utils.LLCSegmentName; | ||
import org.apache.pinot.core.data.manager.InstanceDataManager; | ||
import org.apache.pinot.core.data.manager.realtime.LLRealtimeSegmentDataManager; | ||
import org.apache.pinot.segment.local.data.manager.SegmentDataManager; | ||
import org.apache.pinot.segment.local.data.manager.TableDataManager; | ||
import org.apache.pinot.spi.config.table.TableType; | ||
import org.apache.pinot.spi.stream.StreamPartitionMsgOffset; | ||
import org.apache.pinot.spi.utils.builder.TableNameBuilder; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
|
||
/** | ||
* This class is used at startup time to have a more accurate estimate of the catchup period in which no query execution | ||
* happens and consumers try to catch up to the latest messages available in streams. | ||
* To achieve this, every time status check is called - {@link #getNumConsumingSegmentsNotReachedTheirLatestOffset} - | ||
* for each consuming segment, we check if segment's latest ingested offset has reached the latest stream offset that's | ||
* fetched once at startup time. | ||
*/ | ||
public class OffsetBasedConsumptionStatusChecker { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(OffsetBasedConsumptionStatusChecker.class); | ||
|
||
// constructor parameters | ||
private final InstanceDataManager _instanceDataManager; | ||
private final Set<String> _consumingSegments; | ||
|
||
// helper variable | ||
private final Set<String> _caughtUpSegments = new HashSet<>(); | ||
|
||
public OffsetBasedConsumptionStatusChecker(InstanceDataManager instanceDataManager, Set<String> consumingSegments) { | ||
_instanceDataManager = instanceDataManager; | ||
_consumingSegments = consumingSegments; | ||
} | ||
|
||
public int getNumConsumingSegmentsNotReachedTheirLatestOffset() { | ||
for (String segName : _consumingSegments) { | ||
if (_caughtUpSegments.contains(segName)) { | ||
continue; | ||
} | ||
TableDataManager tableDataManager = getTableDataManager(segName); | ||
if (tableDataManager == null) { | ||
LOGGER.info("TableDataManager is not yet setup for segment {}. Will check consumption status later", segName); | ||
continue; | ||
} | ||
SegmentDataManager segmentDataManager = null; | ||
try { | ||
segmentDataManager = tableDataManager.acquireSegment(segName); | ||
if (segmentDataManager == null) { | ||
LOGGER | ||
.info("SegmentDataManager is not yet setup for segment {}. Will check consumption status later", segName); | ||
continue; | ||
} | ||
if (!(segmentDataManager instanceof LLRealtimeSegmentDataManager)) { | ||
// There's a possibility that a consuming segment has converted to a committed segment. If that's the case, | ||
// segment data manager will not be of type LLRealtime. | ||
LOGGER.info("Segment {} is already committed and is considered caught up.", segName); | ||
_caughtUpSegments.add(segName); | ||
continue; | ||
} | ||
LLRealtimeSegmentDataManager rtSegmentDataManager = (LLRealtimeSegmentDataManager) segmentDataManager; | ||
StreamPartitionMsgOffset latestIngestedOffset = rtSegmentDataManager.getCurrentOffset(); | ||
StreamPartitionMsgOffset latestStreamOffset = rtSegmentDataManager.getLatestStreamOffsetAtStartupTime(); | ||
if (latestStreamOffset == null || latestIngestedOffset == null) { | ||
LOGGER.info("Null offset found for segment {} - latest stream offset: {}, latest ingested offset: {}. " | ||
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 seems these message will be printed per segment. How about collecting all the segment names and group them into different categories, so that they will be printed once once? 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. Good suggestion, but we shouldn't need these logs. We should rely on the service status message. If we want to debug some specific partitions/tables, these logs will be used. Because of that, I think it's better to keep the code simple and don't add extra logic for logging purposes. |
||
+ "Will check consumption status later", segName, latestStreamOffset, latestIngestedOffset); | ||
continue; | ||
} | ||
if (latestIngestedOffset.compareTo(latestStreamOffset) < 0) { | ||
LOGGER.info("Latest ingested offset {} in segment {} is smaller than stream latest available offset {} ", | ||
latestIngestedOffset, segName, latestStreamOffset); | ||
continue; | ||
} | ||
LOGGER.info("Segment {} with latest ingested offset {} has caught up to the latest stream offset {}", segName, | ||
latestIngestedOffset, latestStreamOffset); | ||
_caughtUpSegments.add(segName); | ||
} finally { | ||
if (segmentDataManager != null) { | ||
tableDataManager.releaseSegment(segmentDataManager); | ||
} | ||
} | ||
} | ||
return _consumingSegments.size() - _caughtUpSegments.size(); | ||
} | ||
|
||
private TableDataManager getTableDataManager(String segmentName) { | ||
LLCSegmentName llcSegmentName = new LLCSegmentName(segmentName); | ||
String tableName = llcSegmentName.getTableName(); | ||
String tableNameWithType = TableNameBuilder.forType(TableType.REALTIME).tableNameWithType(tableName); | ||
return _instanceDataManager.getTableDataManager(tableNameWithType); | ||
} | ||
} |
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.
Shouldn't we uncomment it here?
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.
We'll uncomment this part after we analyzed the performance of the new status checker. It's explained in the TODO part a few lines above.