Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2764#discussion_r209044589
--- 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 --
Right, that makes sense. I don't think it requires reordering though, the
sequence you posted can happen without the compiler reordering anything.
I don't think it really hurts anything to have this race, but maybe we
should consider removing the longest update in the gauge. Unless the scheduler
is outright hanging, I'm not sure what the value is for us to report a partial
measurement like that?
---
