smiklosovic commented on code in PR #2983:
URL: https://github.com/apache/cassandra/pull/2983#discussion_r1428941370
##########
src/java/org/apache/cassandra/service/StartupChecks.java:
##########
@@ -187,6 +191,66 @@ 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<Semver> affectedKernels = Range.closedOpen(
+ new Semver("6.1.64", Semver.SemverType.LOOSE), new Semver("6.1.66",
Semver.SemverType.LOOSE));
+
+ private final Set<String> affectedFileSystemTypes = Set.of("ext4");
+
+ @Override
+ public void execute(StartupChecksOptions startupChecksOptions) throws
StartupException
+ {
+ if (startupChecksOptions.isDisabled(getStartupCheckType()))
+ return;
+
+ if (!FBUtilities.isLinux)
+ return;
+
+ Set<Path> directIOWritePaths = new HashSet<>();
+ if (DatabaseDescriptor.getCommitLogWriteDiskAccessMode() ==
Config.DiskAccessMode.direct)
+ directIOWritePaths.add(new
File(DatabaseDescriptor.getCommitLogLocation()).toPath());
+ // TODO: add data directories when direct IO is supported for
flushing and compaction
+
+ if (!directIOWritePaths.isEmpty() &&
IGNORE_KERNEL_BUG_1057843_CHECK.getBoolean())
Review Comment:
why not to check if `IGNORE_KERNEL_BUG_1057843_CHECK` is true first and then
skip everything else if it is? We are creating a hash map, then checking if
disk access mode is direct, then adding paths to set ... just to throw it all
away when that flag is true.
if `directIOWritePaths` is empty but `IGNORE_KERNEL_BUG_1057843_CHECK` if
false, then this `if` will be if (false && false) so it will not `return` here,
then it will iterate over empty set in the next `for` etc ...
I would just add two more `if`s, just after `FBUtilities.isLinux`, testing
if property flag is true and testing if commit log write disk access mode is
not `direct` and immediately returning. The next logic will then be guaranteed
to iterate over `directIOWritePaths` which will not be empty which seems to
just read easier and makes more sense to me.
--
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]
