You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "tanvipenumudy (via GitHub)" <gi...@apache.org> on 2023/05/05 08:28:49 UTC

[GitHub] [ozone] tanvipenumudy opened a new pull request, #4663: HDDS-8534. Enable asynchronous service logging

tanvipenumudy opened a new pull request, #4663:
URL: https://github.com/apache/ozone/pull/4663

   ## What changes were proposed in this pull request?
   
   Currently, asynchronous service logging is not supported in Ozone. However, we do support asynchronous audit logging. 
   Enabling asynchronous service logging can improve performance by allowing the logging thread to continue executing without waiting for the actual logging operation to complete, reducing the impact of logging on the overall system performance.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-8534
   
   ## How was this patch tested?
   
   (Please explain how this patch was tested. Ex: unit tests, manual tests)
   


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] jojochuang commented on pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #4663:
URL: https://github.com/apache/ozone/pull/4663#issuecomment-1553498937

   Hadoop's use of log4j for audit logging is archaic. We recently made a change to make it easier to migrate to log4j2.
   https://issues.apache.org/jira/browse/HADOOP-18631 (two commits)
   
   Basically, use log4j.properties to control AsyncAppender.


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] tanvipenumudy commented on pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on PR #4663:
URL: https://github.com/apache/ozone/pull/4663#issuecomment-1600471574

   Could you please take a final look at the change, @adoroszlai, @duongkame, @jojochuang, thanks!


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] tanvipenumudy commented on a diff in pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #4663:
URL: https://github.com/apache/ozone/pull/4663#discussion_r1238297310


##########
hadoop-ozone/dist/src/shell/conf/log4j.properties:
##########
@@ -15,7 +15,7 @@
 # limitations under the License.
 
 # Define some default values that can be overridden by system properties
-hadoop.root.logger=INFO,console
+hadoop.root.logger=INFO,console,ASYNCRFA

Review Comment:
   Yes, we could remove `ASYNCRFA` from default and override it via system properties when required. Made the required changes, thank you @adoroszlai!



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] kerneltime commented on a diff in pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #4663:
URL: https://github.com/apache/ozone/pull/4663#discussion_r1186307544


##########
hadoop-ozone/dist/src/shell/conf/log4j.properties:
##########
@@ -15,7 +15,7 @@
 # limitations under the License.
 
 # Define some default values that can be overridden by system properties
-hadoop.root.logger=INFO,console
+hadoop.root.logger=INFO,console,ASYNCRFA

Review Comment:
   is there online documentation for this



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4663:
URL: https://github.com/apache/ozone/pull/4663#issuecomment-1554577062

   Thanks @tanvipenumudy for working on this.  Findbugs is failing, please check.
   
   ```
   M M IS: Inconsistent synchronization of org.apache.hadoop.ozone.utils.AsyncRFAAppender.conversionPattern; locked 50% of time  Unsynchronized access at AsyncRFAAppender.java:[line 106]
   ```
   
   https://github.com/tanvipenumudy/ozone/actions/runs/4891447776/jobs/8731974658#step:6:2283


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4663:
URL: https://github.com/apache/ozone/pull/4663#issuecomment-1602373672

   Thanks @tanvipenumudy for the patch, @duongkame, @jojochuang, @smengcl for the reviews.


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on a diff in pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4663:
URL: https://github.com/apache/ozone/pull/4663#discussion_r1236896363


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/utils/AsyncRFAAppender.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.utils;
+
+import java.io.IOException;
+
+import org.apache.log4j.AsyncAppender;
+import org.apache.log4j.PatternLayout;
+import org.apache.log4j.RollingFileAppender;
+import org.apache.log4j.spi.LoggingEvent;
+
+/**
+ * The AsyncRFAAppender shall take the required parameters for supplying
+ * RollingFileAppender to AsyncAppender.
+ */
+public class AsyncRFAAppender extends AsyncAppender {
+
+  private String maxFileSize = String.valueOf(10 * 1024 * 1024);
+
+  private int maxBackupIndex = 1;
+
+  private String fileName = null;
+
+  private String conversionPattern = null;
+
+  private final Object conversionPatternLock = new Object();

Review Comment:
   `conversionPatternLock` seems to be unnecessary after making all access to `conversionPattern` sync on the appender itself.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/utils/AsyncRFAAppender.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.utils;
+
+import java.io.IOException;
+
+import org.apache.log4j.AsyncAppender;
+import org.apache.log4j.PatternLayout;
+import org.apache.log4j.RollingFileAppender;
+import org.apache.log4j.spi.LoggingEvent;
+
+/**
+ * The AsyncRFAAppender shall take the required parameters for supplying
+ * RollingFileAppender to AsyncAppender.
+ */
+public class AsyncRFAAppender extends AsyncAppender {
+
+  private String maxFileSize = String.valueOf(10 * 1024 * 1024);
+
+  private int maxBackupIndex = 1;
+
+  private String fileName = null;
+
+  private String conversionPattern = null;
+
+  private final Object conversionPatternLock = new Object();
+
+  private boolean blocking = true;
+
+  private int bufferSize = DEFAULT_BUFFER_SIZE;
+
+  private RollingFileAppender rollingFileAppender = null;
+
+  private volatile boolean isRollingFileAppenderAssigned = false;
+
+  @Override
+  public void append(LoggingEvent event) {
+    if (rollingFileAppender == null) {
+      appendRFAToAsyncAppender();
+    }
+    super.append(event);
+  }
+
+  private synchronized void appendRFAToAsyncAppender() {

Review Comment:
   Nit: `createRollingFileAppender` may be a better name.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/utils/AsyncRFAAppender.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.utils;
+
+import java.io.IOException;
+
+import org.apache.log4j.AsyncAppender;
+import org.apache.log4j.PatternLayout;
+import org.apache.log4j.RollingFileAppender;
+import org.apache.log4j.spi.LoggingEvent;
+
+/**
+ * The AsyncRFAAppender shall take the required parameters for supplying
+ * RollingFileAppender to AsyncAppender.
+ */
+public class AsyncRFAAppender extends AsyncAppender {

Review Comment:
   Nit: `RFA` stands for `RollingFileAppender`.  A name like `AsyncRollingFileAppender` would avoid duplicating `Appender`.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/utils/AsyncRFAAppender.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.utils;
+
+import java.io.IOException;
+
+import org.apache.log4j.AsyncAppender;
+import org.apache.log4j.PatternLayout;
+import org.apache.log4j.RollingFileAppender;
+import org.apache.log4j.spi.LoggingEvent;
+
+/**
+ * The AsyncRFAAppender shall take the required parameters for supplying
+ * RollingFileAppender to AsyncAppender.
+ */
+public class AsyncRFAAppender extends AsyncAppender {
+
+  private String maxFileSize = String.valueOf(10 * 1024 * 1024);
+
+  private int maxBackupIndex = 1;
+
+  private String fileName = null;
+
+  private String conversionPattern = null;
+
+  private final Object conversionPatternLock = new Object();
+
+  private boolean blocking = true;
+
+  private int bufferSize = DEFAULT_BUFFER_SIZE;
+
+  private RollingFileAppender rollingFileAppender = null;
+
+  private volatile boolean isRollingFileAppenderAssigned = false;
+
+  @Override
+  public void append(LoggingEvent event) {
+    if (rollingFileAppender == null) {
+      appendRFAToAsyncAppender();
+    }
+    super.append(event);
+  }
+
+  private synchronized void appendRFAToAsyncAppender() {
+    if (!isRollingFileAppenderAssigned) {
+      PatternLayout patternLayout;
+      synchronized (conversionPatternLock) {
+        if (conversionPattern != null) {
+          patternLayout = new PatternLayout(conversionPattern);
+        } else {
+          patternLayout = new PatternLayout();
+        }
+      }
+      try {
+        rollingFileAppender =
+            new RollingFileAppender(patternLayout, fileName, true);
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      }
+      rollingFileAppender.setMaxBackupIndex(maxBackupIndex);
+      rollingFileAppender.setMaxFileSize(maxFileSize);
+      this.addAppender(rollingFileAppender);
+      isRollingFileAppenderAssigned = true;
+      super.setBlocking(blocking);
+      super.setBufferSize(bufferSize);
+    }
+  }
+
+  public String getMaxFileSize() {
+    return maxFileSize;
+  }
+
+  public void setMaxFileSize(String maxFileSize) {
+    this.maxFileSize = maxFileSize;
+  }
+
+  public int getMaxBackupIndex() {
+    return maxBackupIndex;
+  }
+
+  public void setMaxBackupIndex(int maxBackupIndex) {
+    this.maxBackupIndex = maxBackupIndex;
+  }
+
+  public String getFileName() {
+    return fileName;
+  }
+
+  public void setFileName(String fileName) {
+    this.fileName = fileName;
+  }
+
+  public synchronized String getConversionPattern() {
+    return conversionPattern;
+  }
+
+  public synchronized void setConversionPattern(String conversionPattern) {
+    this.conversionPattern = conversionPattern;
+  }

Review Comment:
   Not sure why findbugs reported inconsistent synchronization only for `conversionPattern` but not for other member variables.  I think we should synchronize all get/set methods (or none).



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on a diff in pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #4663:
URL: https://github.com/apache/ozone/pull/4663#discussion_r1238268818


##########
hadoop-ozone/dist/src/shell/conf/log4j.properties:
##########
@@ -15,7 +15,7 @@
 # limitations under the License.
 
 # Define some default values that can be overridden by system properties
-hadoop.root.logger=INFO,console
+hadoop.root.logger=INFO,console,ASYNCRFA

Review Comment:
   One question here: should we really add this new appender by default?  Regular RFA is only defined, but not set.



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] tanvipenumudy commented on a diff in pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #4663:
URL: https://github.com/apache/ozone/pull/4663#discussion_r1236947005


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/utils/AsyncRFAAppender.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.utils;
+
+import java.io.IOException;
+
+import org.apache.log4j.AsyncAppender;
+import org.apache.log4j.PatternLayout;
+import org.apache.log4j.RollingFileAppender;
+import org.apache.log4j.spi.LoggingEvent;
+
+/**
+ * The AsyncRFAAppender shall take the required parameters for supplying
+ * RollingFileAppender to AsyncAppender.
+ */
+public class AsyncRFAAppender extends AsyncAppender {
+
+  private String maxFileSize = String.valueOf(10 * 1024 * 1024);
+
+  private int maxBackupIndex = 1;
+
+  private String fileName = null;
+
+  private String conversionPattern = null;
+
+  private final Object conversionPatternLock = new Object();

Review Comment:
   Thank you, made the changes!



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/utils/AsyncRFAAppender.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.utils;
+
+import java.io.IOException;
+
+import org.apache.log4j.AsyncAppender;
+import org.apache.log4j.PatternLayout;
+import org.apache.log4j.RollingFileAppender;
+import org.apache.log4j.spi.LoggingEvent;
+
+/**
+ * The AsyncRFAAppender shall take the required parameters for supplying
+ * RollingFileAppender to AsyncAppender.
+ */
+public class AsyncRFAAppender extends AsyncAppender {
+
+  private String maxFileSize = String.valueOf(10 * 1024 * 1024);
+
+  private int maxBackupIndex = 1;
+
+  private String fileName = null;
+
+  private String conversionPattern = null;
+
+  private final Object conversionPatternLock = new Object();
+
+  private boolean blocking = true;
+
+  private int bufferSize = DEFAULT_BUFFER_SIZE;
+
+  private RollingFileAppender rollingFileAppender = null;
+
+  private volatile boolean isRollingFileAppenderAssigned = false;
+
+  @Override
+  public void append(LoggingEvent event) {
+    if (rollingFileAppender == null) {
+      appendRFAToAsyncAppender();
+    }
+    super.append(event);
+  }
+
+  private synchronized void appendRFAToAsyncAppender() {
+    if (!isRollingFileAppenderAssigned) {
+      PatternLayout patternLayout;
+      synchronized (conversionPatternLock) {
+        if (conversionPattern != null) {
+          patternLayout = new PatternLayout(conversionPattern);
+        } else {
+          patternLayout = new PatternLayout();
+        }
+      }
+      try {
+        rollingFileAppender =
+            new RollingFileAppender(patternLayout, fileName, true);
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      }
+      rollingFileAppender.setMaxBackupIndex(maxBackupIndex);
+      rollingFileAppender.setMaxFileSize(maxFileSize);
+      this.addAppender(rollingFileAppender);
+      isRollingFileAppenderAssigned = true;
+      super.setBlocking(blocking);
+      super.setBufferSize(bufferSize);
+    }
+  }
+
+  public String getMaxFileSize() {
+    return maxFileSize;
+  }
+
+  public void setMaxFileSize(String maxFileSize) {
+    this.maxFileSize = maxFileSize;
+  }
+
+  public int getMaxBackupIndex() {
+    return maxBackupIndex;
+  }
+
+  public void setMaxBackupIndex(int maxBackupIndex) {
+    this.maxBackupIndex = maxBackupIndex;
+  }
+
+  public String getFileName() {
+    return fileName;
+  }
+
+  public void setFileName(String fileName) {
+    this.fileName = fileName;
+  }
+
+  public synchronized String getConversionPattern() {
+    return conversionPattern;
+  }
+
+  public synchronized void setConversionPattern(String conversionPattern) {
+    this.conversionPattern = conversionPattern;
+  }

Review Comment:
   Thank you, made the changes!



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai merged pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #4663:
URL: https://github.com/apache/ozone/pull/4663


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] tanvipenumudy commented on pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on PR #4663:
URL: https://github.com/apache/ozone/pull/4663#issuecomment-1547519075

   @jojochuang, @smengcl could you please take a look at this as well, thanks.


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] tanvipenumudy commented on pull request #4663: HDDS-8534. Enable asynchronous service logging

Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on PR #4663:
URL: https://github.com/apache/ozone/pull/4663#issuecomment-1539780467

   Thank you @duongkame for the review.
   
   - Thank you for bringing up these points - I think it is a valid point that the service logs typically do not generate a significant volume of logs unless there are unexpected issues.
   - For audit logging in Ozone, I believe it is already set to asynchronous logging by default via the Log4j 2 properties: [om-audit-log4j2.properties](https://github.com/apache/ozone/blob/master/hadoop-ozone/dist/src/shell/conf/om-audit-log4j2.properties#L92) - similarly for s3g, scm and dn.
   - Log4j offers the AsyncAppender class, which allows us to perform logging asynchronously. AsyncRFAAppender offers an advantage over the AsyncAppender when it comes to combining asynchronous logging with the rolling file functionality + it provides any additional configuration options if needed. 
   
   I think we will need to evaluate if going by standard AsyncAppender would suffice and be a simpler implementation when compared to the custom AsyncRFAAppender if we choose to make the service logs asynchronous.


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org