Github user zd-project commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2764#discussion_r209034728
  
    --- Diff: 
storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java ---
    @@ -1984,11 +2074,13 @@ private int fragmentedCpu() {
             Cluster cluster = new Cluster(inimbus, supervisors, 
topoToSchedAssignment, topologies, conf);
             cluster.setStatusMap(idToSchedStatus.get());
     
    -        long beforeSchedule = System.currentTimeMillis();
    +        schedulingStartTime.set(Time.nanoTime());
             scheduler.schedule(topologies, cluster);
    -        long scheduleTimeElapsedMs = System.currentTimeMillis() - 
beforeSchedule;
    -        LOG.debug("Scheduling took {} ms for {} topologies", 
scheduleTimeElapsedMs, topologies.getTopologies().size());
    -        scheduleTopologyTimeMs.update(scheduleTimeElapsedMs);
    +        //Will compiler optimize the order of evalutation and cause race 
condition?
    --- End diff --
    
    If compiler is allowed to hoist code, we can ended up with the case: 
    1) gauge "longest-scheduling-time-ms" acquires scheduling start time 
    2) elpased capture end time, update `schedulingDuration` Timer and 
`longestSchedulingTime`
    3) gauge acquires longest time from `longestSchedulingTime` and `currTime` 
    4) since start time isn't null, program execute statement 
    
    ```java
    longest = currTime - startTime > longest ? currTime - startTime : longest;
    ```
    So it'll be slightly longer than the value of `longestSchedulingTime` for 
this round. Which means we might saw jiggles in gauge value


---

Reply via email to