[GitHub] [flink] Myasuka commented on a change in pull request #18989: [FLINK-26306][state/changelog] Randomly offset materialization

[GitBox <gi...@apache.org>]

Myasuka commented on a change in pull request #18989:
URL: https://github.com/apache/flink/pull/18989#discussion_r823406204



##########
File path: flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/PeriodicMaterializationManager.java
##########
@@ -105,7 +116,7 @@ public void start() {
 
             LOG.info("Task {} starts periodic materialization", subtaskName);
 
-            scheduleNextMaterialization();
+            scheduleNextMaterialization(initialDelay);

Review comment:
       Shall we introduce this another method `scheduleInitialMaterization(initialDelay)` and make all calling of `scheduleNextMaterialization(0)` to `scheduleNextMaterialization()`? I think this looks better.

##########
File path: flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/PeriodicMaterializationManager.java
##########
@@ -96,6 +102,11 @@
                 Executors.newSingleThreadScheduledExecutor(
                         new ExecutorThreadFactory(
                                 "periodic-materialization-scheduler-" + subtaskName));
+
+        this.initialDelay =
+                // randomize initial delay to avoid thundering herd problem
+                MathUtils.murmurHash(Objects.hash(operatorId, subtaskIndex))

Review comment:
       The `operatorId` which comes from `OperatorSubtaskDescriptionText` already contains the information of `subtaskIndex`, why we still need another `subtaskIndex` 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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org