[Java] Modified ClusterClock and ConsensusModuleAgent to better support time travel tests.

[kwbc]

trafficstars

ConsensusModuleAgent generally uses ClusterClock.time() for log timestamps and ClusterClock.timeNanos() for internal time-related tasks. During time travel tests we want adjust/offset the log timestamps only. Modified ConsensusModuleAgent to use ClusterClock.convertToNanos instead of timeUnit.convert to allow the implementations of ClusterClock to differentiate between timestamps to be used in the log versus internal time-keeping. Also modified Election to take both the log timestamp and nano time in several methods to avoid converting back from nanoseconds to log time.

Final note: debated about what to call the log-timestamp parameter. Went with 'timestamp', but perhaps 'now' or 'nowLog' would be better?

[mikeb01]

Overall I'm happy with the change (some small comments mentioned).

One question the addition of ClusterClock::convertToNanos. I'm not sure I completely understand the necessity of it, would it be possible to show with a simple test/use case it's value over calling clusterClock.timeUnit().toNanos() directly?

Final note: debated about what to call the log-timestamp parameter. Went with 'timestamp', but perhaps 'now' or 'nowLog' would be better?

I think timestamp is a bit generic. Not sure what the best name should be (nowClusterUnits is a bit wordy, logTimestamp is another option).

[kwbc]

One question the addition of ClusterClock::convertToNanos. I'm not sure I completely understand the necessity of it, would it be possible to show with a simple test/use case it's value over calling clusterClock.timeUnit().toNanos() directly?

In our time travel implementation of ClusterClock, time() returns epochNanos() + timeTravelOffset, and convertToNanos(time) returns time - timeTravelOffset thus converting the adjusted log timestamp back to real wall-clock time for internal time keeping purposes within the consensus module. Does that make sense?

Would be simpler/clearer if consensus module used ClusterClock for log timestamps (and timers) only, and some other EpochClock for everything else. But that would impose a performance penalty having to call ClusterClock.time and EpochClock.time in the same duty-cycle.

[kwbc]

I think timestamp is a bit generic. Not sure what the best name should be (nowClusterUnits is a bit wordy, logTimestamp is another option).

I like logTimestamp. logClusterUnit is a bit wordy, but more consistent with nowNs and only slightly longer. Anyone else have an opinion?

[kwbc]

Thanks for the code review comments. I've renamed timestamp parameters to logTimestamp in ConsensusModuleAgent and Election -- like how it is consistent with logPosition, etc. And other requested parameter renames.

[mjpt777]

I'll review this Friday or over the weekend.