Maxwell-Guo commented on code in PR #2983:
URL: https://github.com/apache/cassandra/pull/2983#discussion_r1423991544


##########
src/java/org/apache/cassandra/service/StartupChecks.java:
##########
@@ -187,6 +190,58 @@ public void verify(StartupChecksOptions options) throws 
StartupException
         }
     }
 
+    // https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1057843
+    public static final StartupCheck checkKernelBug1057843 = new StartupCheck()

Review Comment:
   I would recommend we just rename the function name to checkKernelStatus / 
checkKernelVersion, 
   check the kernel bug may just be one part of the checkKernelSatstu funtion , 
and we can do other kernel check in the future if needed.



##########
src/java/org/apache/cassandra/service/StartupChecks.java:
##########
@@ -187,6 +190,58 @@ public void verify(StartupChecksOptions options) throws 
StartupException
         }
     }
 
+    // https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1057843
+    public static final StartupCheck checkKernelBug1057843 = new StartupCheck()
+    {
+        private final Range<CassandraVersion> affectedKernels = 
Range.closedOpen(
+        new CassandraVersion("6.1.64").toBaseVersion(), new 
CassandraVersion("6.1.66").toBaseVersion());

Review Comment:
   I have a question. Is it a bit confusing to use the Linux kernel version to 
build the CassandraVersion object?
   
   What about just Implement A linuxKernelVersion to do what we want.



##########
src/java/org/apache/cassandra/utils/FBUtilities.java:
##########
@@ -1320,4 +1368,51 @@ public static void closeQuietly(Object o)
             logger.warn("Closing {} had an unexpected exception", o, e);
         }
     }
+
+    public static CassandraVersion getKernelVersion()
+    {
+        return kernelVersionSupplier.get();
+    }
+
+    @VisibleForTesting
+    static CassandraVersion getKernelVersionFromUname()
+    {
+        if (!isLinux)
+            return null;
+
+        try
+        {
+            String output = exec(Map.of(), Duration.ofSeconds(5), 1024, 1024, 
"uname", "-r");
+
+            if (output.isEmpty())
+                throw new RuntimeException("Error while trying to get kernel 
version, 'uname -r' returned empty output");
+
+            return parseKernelVersion(output);
+        }
+        catch (IOException | TimeoutException e)
+        {
+            throw new RuntimeException("Error while trying to get kernel 
version", e);
+        }
+        catch (InterruptedException e)
+        {
+            Thread.currentThread().interrupt();
+            throw new RuntimeException(e);
+        }
+    }
+
+    @VisibleForTesting
+    static CassandraVersion parseKernelVersion(String outStr)
+    {
+        try (Scanner scanner = new Scanner(outStr))
+        {
+            while (scanner.hasNextLine())
+            {
+                String version = scanner.nextLine().trim();
+                if (version.isBlank() || version.isEmpty())
+                    continue;
+                return new CassandraVersion(version);
+            }
+        }
+        return null;

Review Comment:
   if we implement a KernelVersion class I think we can add a NULL_VERSION 
constant for KernelVersion , and return this NULL_VERSION here instand of null. 
   
   Besides, the function name is parseKernelVersion ,but the return param is 
CassandraVersion ~~~.



##########
src/java/org/apache/cassandra/utils/FBUtilities.java:
##########
@@ -1320,4 +1368,51 @@ public static void closeQuietly(Object o)
             logger.warn("Closing {} had an unexpected exception", o, e);
         }
     }
+
+    public static CassandraVersion getKernelVersion()
+    {
+        return kernelVersionSupplier.get();
+    }
+
+    @VisibleForTesting
+    static CassandraVersion getKernelVersionFromUname()
+    {
+        if (!isLinux)
+            return null;
+
+        try
+        {
+            String output = exec(Map.of(), Duration.ofSeconds(5), 1024, 1024, 
"uname", "-r");
+
+            if (output.isEmpty())
+                throw new RuntimeException("Error while trying to get kernel 
version, 'uname -r' returned empty output");
+
+            return parseKernelVersion(output);
+        }
+        catch (IOException | TimeoutException e)
+        {
+            throw new RuntimeException("Error while trying to get kernel 
version", e);
+        }
+        catch (InterruptedException e)
+        {
+            Thread.currentThread().interrupt();
+            throw new RuntimeException(e);
+        }
+    }
+
+    @VisibleForTesting
+    static CassandraVersion parseKernelVersion(String outStr)
+    {
+        try (Scanner scanner = new Scanner(outStr))
+        {
+            while (scanner.hasNextLine())
+            {
+                String version = scanner.nextLine().trim();
+                if (version.isBlank() || version.isEmpty())
+                    continue;
+                return new CassandraVersion(version);
+            }
+        }
+        return null;

Review Comment:
   if we implement a KernelVersion class I think we can add a NULL_VERSION 
constant for KernelVersion , and return this NULL_VERSION here instand of null. 
   
   Besides, the function name is parseKernelVersion ,but the return param is 
CassandraVersion ~~~.



##########
src/java/org/apache/cassandra/utils/FBUtilities.java:
##########
@@ -1080,6 +1093,41 @@ public static void exec(ProcessBuilder pb) throws 
IOException
         }
     }
 
+    public static String exec(Map<String, String> env, Duration timeout, int 
outBufSize, int errBufSize, String... cmd) throws IOException, 
TimeoutException, InterruptedException
+    {
+        ProcessBuilder processBuilder = new ProcessBuilder(cmd);
+        processBuilder.environment().putAll(env);
+        Process process = processBuilder.start();
+        try (DataOutputBuffer err = new DataOutputBuffer();
+             DataOutputBuffer out = new DataOutputBuffer();
+             OutputStream overflowSink = OutputStream.nullOutputStream())
+        {
+            boolean completed = process.waitFor(timeout.toMillis(), 
TimeUnit.MILLISECONDS);
+
+            copy(process.getInputStream(), out, outBufSize);
+            long outOverflow = 
process.getInputStream().transferTo(overflowSink);
+
+            copy(process.getErrorStream(), err, errBufSize);
+            long errOverflow = 
process.getErrorStream().transferTo(overflowSink);
+
+            if (!completed)
+            {
+                process.destroyForcibly();
+                logger.warn("Command {} did not complete in {}, killed 
forcibly:\noutput:\n{}\n(truncated {} bytes)\nerror:\n{}\n(truncated {} bytes)",
+                            Arrays.toString(cmd), timeout, out.asString(), 
outOverflow, err.asString(), errOverflow);
+                throw new TimeoutException("Command " + Arrays.toString(cmd) + 
" did not complete in " + timeout);
+            }
+            int r = process.exitValue();
+            if (r != 0)
+            {
+                logger.warn("Command {} failed with exit code 
{}:\noutput:\n{}\n(truncated {} bytes)\nerror:\n{}\n(truncated {} bytes)",

Review Comment:
   should we change the logging level to error ?



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