JeetKunDoug commented on code in PR #172:
URL: https://github.com/apache/cassandra-sidecar/pull/172#discussion_r1915344587


##########
server/src/main/java/org/apache/cassandra/sidecar/config/ServiceConfiguration.java:
##########
@@ -92,7 +92,7 @@ default List<SocketAddress> listenSocketAddresses()
     /**
      * @return the maximum time skew allowed between the server and the client
      */
-    int allowableSkewInMinutes();
+    SecondBoundConfiguration allowableTimeSkew();

Review Comment:
   It should be minutes btw... this isn't a replacement for NTP, it's a 
guardrail to make sure that the Spark workers aren't so far skewed from C* that 
deletes and TTLs do weird things (since Spark writes the row timestamps in the 
Analytics library, where we consume this endpoint).  It is both possible and 
acceptable for there to be several minutes of skew in environments that don't 
have tightly controlled NTP sync, and specifying this in seconds really doesn't 
make any sense (is 65 seconds acceptable, but 66 not?)
   
   That said, I don't really mind that it's _set_ in seconds, but the 
expectation that somehow the resolution of seconds is meaningful here isn't 
realistic.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to