[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1693: IGNITE-18839 HybridTimestamp and its usages refactored to use 8-bytes representation

["sanpwc (via GitHub)" <gi...@apache.org>]

sanpwc commented on code in PR #1693:
URL: https://github.com/apache/ignite-3/pull/1693#discussion_r1180485044


##########
modules/core/src/main/java/org/apache/ignite/internal/hlc/HybridTimestamp.java:
##########
@@ -27,26 +28,34 @@ public final class HybridTimestamp implements Comparable<HybridTimestamp>, Seria
     /** Serial version UID. */
     private static final long serialVersionUID = 2459861612869605904L;

Review Comment:
   Formally, it should be changed.



##########
modules/core/src/main/java/org/apache/ignite/internal/hlc/HybridClockImpl.java:
##########
@@ -55,47 +57,52 @@ public HybridClockImpl() {
      * @param initialTime Initial time.
      */
     public HybridClockImpl(HybridTimestamp initialTime) {

Review Comment:
   Seems that given method is never used. Why do we need it?



##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/core/Replicator.java:
##########
@@ -743,7 +744,7 @@ private void sendEmptyEntries(final boolean isHeartbeat) {
     private void sendEmptyEntries(final boolean isHeartbeat,
         final RpcResponseClosure<AppendEntriesResponse> heartBeatClosure) {
         final AppendEntriesRequestBuilder rb = raftOptions.getRaftMessagesFactory().appendEntriesRequest();
-        rb.timestamp(options.getNode().clockNow());
+        rb.timestampLong(nullableLongTime(options.getNode().clockNow()));

Review Comment:
   Not sure that I get an idea of why we use nullable in some places and "non-nullable" in another ones. E.g. why it's possible to have null in this particular place?



##########
modules/core/src/main/java/org/apache/ignite/internal/hlc/HybridClock.java:
##########
@@ -21,6 +21,13 @@
  * A Hybrid Logical Clock.
  */
 public interface HybridClock {
+    /**

Review Comment:
   What about checking logical part overflow? Not sure that it's worth it though.



-- 
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: notifications-unsubscribe@ignite.apache.org

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