Jackie-Jiang commented on code in PR #10121:
URL: https://github.com/apache/pinot/pull/10121#discussion_r1083128944
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/IngestionDelayTracker.java:
##########
@@ -196,21 +202,28 @@ void setClock(Clock clock) {
* Called by LLRealTimeSegmentDataManagers to post ingestion time updates to
this tracker class.
*
* @param ingestionTimeMs ingestion time being recorded.
+ * @param creationTimeMs event creation time.
* @param partitionGroupId partition ID for which this ingestion time is
being recorded.
*/
- public void updateIngestionDelay(long ingestionTimeMs, int partitionGroupId)
{
+ public void updateIngestionDelay(long ingestionTimeMs, long creationTimeMs,
int partitionGroupId) {
// Store new measure and wipe old one for this partition
- // TODO: see if we can install gauges after the server is ready.
if (!_isServerReadyToServeQueries.get()) {
// Do not update the ingestion delay metrics during server startup period
return;
}
- Long previousMeasure = _partitionToIngestionTimeMsMap.put(partitionGroupId,
- ingestionTimeMs);
+ IngestionTimestamps previousMeasure =
_partitionToIngestionTimestampsMap.put(partitionGroupId,
+ new IngestionTimestamps(ingestionTimeMs, creationTimeMs));
if (previousMeasure == null) {
// First time we start tracking a partition we should start tracking it
via metric
_serverMetrics.setOrUpdatePartitionGauge(_metricName, partitionGroupId,
ServerGauge.REALTIME_INGESTION_DELAY_MS, () ->
getPartitionIngestionDelayMs(partitionGroupId));
+ if (creationTimeMs != Long.MIN_VALUE) {
Review Comment:
Suggest using `<=0` to be more flexible
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/IngestionDelayTracker.java:
##########
@@ -141,10 +150,7 @@ public IngestionDelayTracker(ServerMetrics serverMetrics,
String tableNameWithTy
*
* @param ingestionTimeMs original ingestion time in milliseconds.
*/
- private long getIngestionDelayMs(Long ingestionTimeMs) {
- if (ingestionTimeMs == null) {
- return 0; // return 0 when not initialized
- }
+ private long getIngestionDelayMs(long ingestionTimeMs) {
Review Comment:
Suggest adding a checking to prevent invalid value:
```
if (ingestionTimeMs <= 0) {
return 0;
}
```
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
