jugomezv commented on code in PR #9994:
URL: https://github.com/apache/pinot/pull/9994#discussion_r1061886497
##########
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:
After implementing both approaches I think we are likely better off with
individual methods, reason being are the following:
1.-There is precedent on handling transitions in individual methods. See
SegmentOnlineOfflineStateModeFactory.
2.-At this point we are adding only two interfaces, if we have to add more
we can revisit then.
3.-Keeping a single method for transition makes the code simple, no throw,
checks on states etc.
So I will roll this change back, @navina hopefully you are ok with 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
