[GitHub] [bookkeeper] sijie commented on a change in pull request #2198: Fix the log level of not support Sse42Crc32C

[GitBox <gi...@apache.org>]

sijie commented on a change in pull request #2198: Fix the log level of not support Sse42Crc32C
URL: https://github.com/apache/bookkeeper/pull/2198#discussion_r342933517
 
 

 ##########
 File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/checksum/CRC32CDigestManager.java
 ##########
 @@ -41,8 +43,14 @@ protected MutableInt initialValue() throws Exception {
 
     public CRC32CDigestManager(long ledgerId, boolean useV2Protocol, ByteBufAllocator allocator) {
         super(ledgerId, useV2Protocol, allocator);
+
+        if (!isSupported) {
 
 Review comment:
   I don't think `isSupported` is a good name. it is actually a confusing name in this case. because the flag isn't used for telling whether CRC32C is support. The flag should be used for controlling whether the error log message is printed or not.
   
   I would suggest renaming it to "nonSupportedMessagePrinted" and make the default value as `false`.
   
   Then you can change the logic to:
   
   ```
   if (!Sse42Crc32C.isSupported() && !nonSupportedMessagePrinted) {
       log.error("Sse42Crc32C is not supported, will use a slower CRC32C implementation.");
       nonSupportedMessagePrinted = true;
   }
   ```
   

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


With regards,
Apache Git Services