navina commented on code in PR #9994:
URL: https://github.com/apache/pinot/pull/9994#discussion_r1061806303
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java:
##########
@@ -212,6 +216,83 @@ protected void doShutdown() {
if (_leaseExtender != null) {
_leaseExtender.shutDown();
}
+ // Make sure we do metric cleanup when we shut down the table.
+ _ingestionDelayTracker.shutdown();
+ }
+
+ /*
+ * Method to handle CONSUMING->DROPPED transition.
+ *
+ * @param partitionGroupId Partition id that we must stop tracking on this
server.
+ */
+ private void stopTrackingPartitionDelay(int partitionGroupId) {
+
_ingestionDelayTracker.stopTrackingPartitionIngestionDelay(partitionGroupId);
+ }
+
+ /*
+ * Method to handle CONSUMING->ONLINE transition.
+ * If no new ingestion is noticed for this segment in some timeout, we will
read
+ * ideal state to verify the partition is still hosted in this server.
+ *
+ * @param partitionGroupId partition id of partition to be verified as
hosted by this server.
+ */
+ private void markPartitionForVerification(int partitionGroupId) {
+ _ingestionDelayTracker.markPartitionForConfirmation(partitionGroupId);
+ }
+
+ /*
+ * Method used by LLRealtimeSegmentManagers to update their partition delays
+ *
+ * @param ingestionDelayMs Ingestion delay being reported.
+ * @param currentTimeMs Timestamp of the measure being provided, i.e. when
this delay was computed.
+ * @param partitionGroupId Partition ID for which delay is being updated.
+ */
+ public void updateIngestionDelay(long ingestionDelayMs, long currenTimeMs,
int partitionGroupId) {
+ _ingestionDelayTracker.updateIngestionDelay(ingestionDelayMs,
currenTimeMs, partitionGroupId);
+ }
+
+ /*
+ * Method to handle supported transitions of segments states for this table.
+ * Supported transitions include:
+ *
+ * CONSUMING -> ONLINE:
+ * We mark partitions for verification against ideal state when we do not
see a consuming segment for some time
+ * for that partition. The idea is to remove the related metrics when the
partition moves from the current server.
+ * CONSUMING -> DROPPED:
+ * We stop tracking partitions whose segments are dropped.
+ *
+ * @param segmentNameStr name of segment which is transitioning state.
+ * @param fromState state from which the segment is transitioning.
+ * @param toState state to which the segment is transitioning to.
+ */
+ @Override
+ public void onSegmentStateTransition(String segmentNameStr, SegmentState
fromState, SegmentState toState) {
Review Comment:
I don't think we should be adding a new method for each state transition.
Then, the number of methods here would potentially grow to the number of state
transitions.
There are alternatives if you want to avoid if statements (throwing it out
there: define constants for each transition like `CONSUMING_TO_ONLINE` ,
`DROPPED_ENDSTATE` ). It will improve readability while keeping a check on the
number of new interface methods.
--
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
