smiklosovic commented on code in PR #2046:
URL: https://github.com/apache/cassandra/pull/2046#discussion_r1053481885


##########
src/java/org/apache/cassandra/gms/FailureDetector.java:
##########
@@ -93,16 +91,12 @@ public FailureDetector()
 
     private static long getInitialValue()
     {
-        String newvalue = System.getProperty("cassandra.fd_initial_value_ms");

Review Comment:
   I am not completely sure we can do this. If we move 
`Gossiper.intervalInMillis * 2` to `CassandraRelevantProperties`, is not it 
true that Gossiper's static variables / block will be initialized too? What are 
the consequences of doing that?
   
   Consider this:
   
   ````
   public class MyTest
   {
       public static final String[] ARGS = {};
   
       @Test
       public void test()
       {
           MyTest.main(ARGS);
       }
   
       public static void main(String[] args)
       {
           System.out.println(MyEnum.values().length);
       }
   
       public enum MyEnum
       {
           ENUM_A("MY_ENUM", Gossiper.b * 2);
   
           private String key;
           private int value;
   
           private MyEnum(String key, int value)
           {
               this.key = key;
               this.value = value;
           }
       }
   
       public static class Gossiper
       {
           static
           {
               System.out.println("in static initialiser of Gossiper");
           }
   
           public static int a = initA();
   
           public static int b = initB();
   
           public static int initA()
           {
               System.out.println("in Gossiper.initA");
               return 1;
           }
   
           public static int initB()
           {
               System.out.println("in Gossiper.initB");
               return 5;
           }
       }
   }
   
   ````
   
   This will print:
   
   ````
   in static initialiser of Gossiper
   in Gossiper.initA
   in Gossiper.initB
   1
   ````
   
   This basically means that by calling `FD_INITIAL_VALUE_MS`, it will eagerly 
call static initializer in Gossiper too when that enum is being initialized. By 
doing that, we are changing the previous behavior.
   
   I would rewrite this in such a way that `Gossiper.intervalInMillis` would be 
called only in case it has to be
   
   ````
       private static long getInitialValue()
       {
           String value = FD_INITIAL_VALUE_MS.getString();
           if (value == null)
               return Gossiper.intervalInMillis * 2;
   
           long newValue = Long.parseLong(value);
   
           if (newValue != Gossiper.intervalInMillis * 2)
               logger.info("Overriding {} from {}ms to {}ms",
                   FD_INITIAL_VALUE_MS.getKey(), defaultValue, newValue);
   
           return newValue;
       }
   ````



-- 
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