You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/11/27 08:49:55 UTC

[GitHub] [ignite] nyshubin opened a new pull request #8503: IGNITE-13680 maxSwappiness declared as int in suggestions

nyshubin opened a new pull request #8503:
URL: https://github.com/apache/ignite/pull/8503


   IGNITE-13680 maxSwappiness declared as int in OsConfigurationSuggestions.java


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

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



[GitHub] [ignite] alamar commented on a change in pull request #8503: IGNITE-13680 maxSwappiness declared as int in suggestions

Posted by GitBox <gi...@apache.org>.
alamar commented on a change in pull request #8503:
URL: https://github.com/apache/ignite/pull/8503#discussion_r539413185



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/suggestions/OsConfigurationSuggestions.java
##########
@@ -60,12 +60,12 @@
     public static synchronized List<String> getSuggestions() {
         List<String> suggestions = new ArrayList<>();
 
-        if (U.isRedHat()) {
-            String value;
-            String expected = "500";
+        if (U.isLinux()) {
+            String val;
+            String exp = "500";
 
-            boolean dwcParamFlag = (value = readVmParam(DIRTY_WRITEBACK_CENTISECS)) != null && !value.equals(expected);
-            boolean decParamFlag = (value = readVmParam(DIRTY_EXPIRE_CENTISECS)) != null && !value.equals(expected);
+            boolean dwcParamFlag = (val = readVmParam(DIRTY_WRITEBACK_CENTISECS)) != null && !val.equals(exp);

Review comment:
       Can we parse the value as int and do a "less than" comparison? val < exp (500)
   Otherwise we may be telling people to slow down their writeback. Applies for both.

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/suggestions/OsConfigurationSuggestions.java
##########
@@ -74,28 +74,28 @@
                     (dwcParamFlag && decParamFlag ? " and " : ""),
                     (decParamFlag ? "vm." + DIRTY_EXPIRE_CENTISECS : ""),
                     (dwcParamFlag && decParamFlag ? "s" : ""),
-                    expected));
+                    exp));
 
-            if ((value = readVmParam(SWAPPINESS)) != null) {
+            if ((val = readVmParam(SWAPPINESS)) != null) {
                 try {
-                    double maxSwappiness = 10.0;
+                    int maxSwappiness = 10;
 
-                    if (Float.parseFloat(value) > maxSwappiness)
-                        suggestions.add(String.format("Reduce pages swapping ratio (set vm.%s=%f or less)", SWAPPINESS,
+                    if (Integer.parseInt(val) > maxSwappiness)
+                        suggestions.add(String.format("Reduce pages swapping ratio (set vm.%s=%d or less)", SWAPPINESS,
                                                       maxSwappiness));
                 }
                 catch (NumberFormatException ignored) {
                     // OS param not parsable as a number
                 }
             }
 
-            if ((value = readVmParam(ZONE_RECLAIM_MODE)) != null && !value.equals(expected = "0"))
+            if ((val = readVmParam(ZONE_RECLAIM_MODE)) != null && !val.equals(exp = "0"))
                 suggestions.add(String.format("Disable NUMA memory reclaim (set vm.%s=%s)", ZONE_RECLAIM_MODE,
-                    expected));
+                    exp));
 
-            if ((value = readVmParam(EXTRA_FREE_KBYTES)) != null && !value.equals(expected = "1240000"))
+            if ((val = readVmParam(EXTRA_FREE_KBYTES)) != null && !val.equals(exp = "1240000"))

Review comment:
       Can we also get rid of this check (and corresponding constant?) It seems to be a proprietary Red Hat property abandoned a long time ago, and the hard-coded value is extremely suspicious.




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

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



[GitHub] [ignite] nyshubin commented on a change in pull request #8503: IGNITE-13680 maxSwappiness declared as int in suggestions

Posted by GitBox <gi...@apache.org>.
nyshubin commented on a change in pull request #8503:
URL: https://github.com/apache/ignite/pull/8503#discussion_r549232650



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/suggestions/OsConfigurationSuggestions.java
##########
@@ -74,28 +74,28 @@
                     (dwcParamFlag && decParamFlag ? " and " : ""),
                     (decParamFlag ? "vm." + DIRTY_EXPIRE_CENTISECS : ""),
                     (dwcParamFlag && decParamFlag ? "s" : ""),
-                    expected));
+                    exp));
 
-            if ((value = readVmParam(SWAPPINESS)) != null) {
+            if ((val = readVmParam(SWAPPINESS)) != null) {
                 try {
-                    double maxSwappiness = 10.0;
+                    int maxSwappiness = 10;
 
-                    if (Float.parseFloat(value) > maxSwappiness)
-                        suggestions.add(String.format("Reduce pages swapping ratio (set vm.%s=%f or less)", SWAPPINESS,
+                    if (Integer.parseInt(val) > maxSwappiness)
+                        suggestions.add(String.format("Reduce pages swapping ratio (set vm.%s=%d or less)", SWAPPINESS,
                                                       maxSwappiness));
                 }
                 catch (NumberFormatException ignored) {
                     // OS param not parsable as a number
                 }
             }
 
-            if ((value = readVmParam(ZONE_RECLAIM_MODE)) != null && !value.equals(expected = "0"))
+            if ((val = readVmParam(ZONE_RECLAIM_MODE)) != null && !val.equals(exp = "0"))
                 suggestions.add(String.format("Disable NUMA memory reclaim (set vm.%s=%s)", ZONE_RECLAIM_MODE,
-                    expected));
+                    exp));
 
-            if ((value = readVmParam(EXTRA_FREE_KBYTES)) != null && !value.equals(expected = "1240000"))
+            if ((val = readVmParam(EXTRA_FREE_KBYTES)) != null && !val.equals(exp = "1240000"))

Review comment:
       replaced this check with int comparison, but did not remove it completely, because gridgain doc suggests:
   To work around the effects caused by page memory reclamation on Linux, add extra bytes between wmark_min and wmark_low with /proc/sys/vm/extra_free_kbytes




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

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



[GitHub] [ignite] asfgit closed pull request #8503: IGNITE-13680 maxSwappiness declared as int in suggestions

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #8503:
URL: https://github.com/apache/ignite/pull/8503


   


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

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



[GitHub] [ignite] alamar commented on a change in pull request #8503: IGNITE-13680 maxSwappiness declared as int in suggestions

Posted by GitBox <gi...@apache.org>.
alamar commented on a change in pull request #8503:
URL: https://github.com/apache/ignite/pull/8503#discussion_r533990699



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/suggestions/OsConfigurationSuggestions.java
##########
@@ -61,11 +61,11 @@
         List<String> suggestions = new ArrayList<>();
 
         if (U.isRedHat()) {

Review comment:
       Can we perhaps do this for all Linux VMs and not just Red Hat? I have Ubuntu but all of these VM params are there.




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

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



[GitHub] [ignite] nyshubin commented on a change in pull request #8503: IGNITE-13680 maxSwappiness declared as int in suggestions

Posted by GitBox <gi...@apache.org>.
nyshubin commented on a change in pull request #8503:
URL: https://github.com/apache/ignite/pull/8503#discussion_r549232015



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/suggestions/OsConfigurationSuggestions.java
##########
@@ -60,12 +60,12 @@
     public static synchronized List<String> getSuggestions() {
         List<String> suggestions = new ArrayList<>();
 
-        if (U.isRedHat()) {
-            String value;
-            String expected = "500";
+        if (U.isLinux()) {
+            String val;
+            String exp = "500";
 
-            boolean dwcParamFlag = (value = readVmParam(DIRTY_WRITEBACK_CENTISECS)) != null && !value.equals(expected);
-            boolean decParamFlag = (value = readVmParam(DIRTY_EXPIRE_CENTISECS)) != null && !value.equals(expected);
+            boolean dwcParamFlag = (val = readVmParam(DIRTY_WRITEBACK_CENTISECS)) != null && !val.equals(exp);

Review comment:
       Cast exp and val to int and replaced .equals comparison with val > exp (500)
   The gridgain guide suggests: As a solution, you can try changing the page flushing interval from the default 30 seconds to 5 seconds




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

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