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


##########
src/java/org/apache/cassandra/service/StartupChecks.java:
##########
@@ -417,14 +417,26 @@ public void execute(StartupChecksOptions options) throws 
StartupException
         }
     };
 
-    public static final StartupCheck initSigarLibrary = new StartupCheck()
+    public static final StartupCheck checkProcessEnvironment = new 
StartupCheck()
     {
         @Override
         public void execute(StartupChecksOptions options)
         {
             if (options.isDisabled(getStartupCheckType()))
                 return;
-            SigarLibrary.instance.warnIfRunningInDegradedMode();
+
+            Optional<String> degradations = new SystemInfo().isDegraded();
+
+            if (degradations.isPresent())
+            {
+                String message = "Cassandra server running in degraded mode. " 
+ degradations.get();
+                if 
(CassandraRelevantProperties.TEST_IGNORE_PROCESS_ENVIRONMENT_CHECK.getBoolean())
+                    logger.warn(message);
+                else
+                    logger.info(message);
+            }
+            else
+                logger.info("Checked OS settings and found them configured for 
optimal performance.");

Review Comment:
   @jacek-lewandowski  @Claudenw 
   
   I am fine with informational log when all is fine as it was before 
(otherwise we are changing the behavior here). Maybe  somebody parses their 
logs on this message and we will remove it so they will have nothing to grep on?
   
   Also, if we indeed want to skip the system check, how would we go around 
this? I think that `TEST_IGNORE_PROCESS_ENVIRONMENT_CHECK` in its current form 
is not doing anything useful. It is not like if it is set to true, the check 
itself would be skipped ... 
   
   Some time ago, I made startup checks to be configurable (CASSANDRA-17220). 
So if we indeed want to make this check skippable, we would need to change its 
StartupCheckType so this
   
               if (options.isDisabled(getStartupCheckType()))
                   return;
   
   will evaluate to `true`.
   
   You could do this in cassandra.yaml then
   
           startup_checks:
               check_process_environment:
                   enabled: false
   
   So, if we do not want to ever skip this check, then whole TEST_IGNORE... 
property should be just removed. If we want to have a possibility to skip it, 
then we should make it configurable to set `enabled: false`. We could also 
introduce a system property (instead of `TEST_IGNORE...` and return from 
check's execution when it is `true`. That has same functionality as the change 
in cassandra.yaml but it is easier to set it e.g. for some tests when we do not 
need to mess with setting that in some testing cassandra.yaml file. Currently, 
I do not see we are in fact skipping this check in tests so this capability 
could be just omitted. 



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