[GitHub] [logging-log4j2] vy commented on a diff in pull request #973: LOG4J2-3558: make default priority configurable

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

vy commented on code in PR #973:
URL: https://github.com/apache/logging-log4j2/pull/973#discussion_r946927369


##########
log4j-core/src/main/java/org/apache/logging/log4j/core/util/Log4jThreadFactory.java:
##########
@@ -26,6 +28,7 @@
  * @since 2.7
  */
 public class Log4jThreadFactory implements ThreadFactory {
+    public static final String THREAD_PRIORITY = "log4j2.thread.priority";

Review Comment:
   ```suggestion
   ```



##########
log4j-core/src/main/java/org/apache/logging/log4j/core/util/Log4jThreadFactory.java:
##########
@@ -94,4 +109,8 @@ public Thread newThread(final Runnable runnable) {
         return thread;
     }
 
+    private static int getThreadPriority() {
+        int result = PropertiesUtil.getProperties().getIntegerProperty(THREAD_PRIORITY, Thread.NORM_PRIORITY);
+        return Math.min(Math.max(Thread.MIN_PRIORITY, result), Thread.MAX_PRIORITY);

Review Comment:
   I would do the following instead:
   
   ```suggestion
           final String property = "log4j2.thread.priority";
           int priority = PropertiesUtil.getProperties().getIntegerProperty(property, Thread.NORM_PRIORITY);
           if (priority < Thread.MIN_PRIORITY || priority > Thread.MAX_PRIORITY) {
               StatusLogger.getLogger().warn("invalid {}: {}", property, priority);
               priority = Thread.NORM_PRIORITY;
           }
           return priority;
   ```
   
   There are many other places in Log4j where threads are created. We might want to choose another name for the property.



-- 
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: notifications-unsubscribe@logging.apache.org

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