You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ptlrs (via GitHub)" <gi...@apache.org> on 2025/10/31 18:48:55 UTC
[PR] HDDS-13794: Use synchronized logger for auditing tasks [ozone]
ptlrs opened a new pull request, #9229:
URL: https://github.com/apache/ozone/pull/9229
## What changes were proposed in this pull request?
This PR updates audit loggers to use synchronized loggers instead of async loggers.
1. Update log4 configuration files to remove async loggers
2. Update JVM options to remove the async contextSelector. This would otherwise make all loggers in that JVM async.
3. Update the tests
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13794
## How was this patch tested?
CI: https://github.com/ptlrs/ozone/actions/runs/18575985039
--
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
Re: [PR] HDDS-13794. Use synchronized logger for auditing tasks [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on code in PR #9229:
URL: https://github.com/apache/ozone/pull/9229#discussion_r2482995671
##########
hadoop-ozone/dist/src/shell/ozone/ozone:
##########
@@ -107,7 +109,7 @@ function ozonecmd_case
ozone_deprecate_envvar HDDS_DN_OPTS OZONE_DATANODE_OPTS
OZONE_DATANODE_OPTS="${RATIS_OPTS} ${OZONE_DATANODE_OPTS}"
OZONE_DATANODE_OPTS="-Dlog4j.configurationFile=${OZONE_CONF_DIR}/dn-audit-log4j2.properties,${OZONE_CONF_DIR}/dn-container-log4j2.properties ${OZONE_DATANODE_OPTS}"
- OZONE_DATANODE_OPTS="-Dlog4j2.contextSelector=org.apache.logging.log4j.core.async.AsyncLoggerContextSelector ${OZONE_DATANODE_OPTS} ${OZONE_MODULE_ACCESS_ARGS}"
+ OZONE_DATANODE_OPTS="${OZONE_DATANODE_OPTS} ${OZONE_MODULE_ACCESS_ARGS}"
Review Comment:
Yes, this is already happening. We use two versions of Log4J.
Log4Jv2 was used only for auditing. This PR removes the AsyncLogger.
Log4Jv1's AsyncAppender is used for all regular logging including dependencies and libraries.
--
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
Re: [PR] HDDS-13794. Use synchronized logger for auditing tasks [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on code in PR #9229:
URL: https://github.com/apache/ozone/pull/9229#discussion_r2482999620
##########
hadoop-ozone/s3gateway/src/test/resources/auditlog.properties:
##########
@@ -1,18 +1,18 @@
#
# 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>
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
Review Comment:
https://issues.apache.org/jira/browse/HDDS-12251 updated the license format but it appears that the new license text was updated only for Java files.
Since I use Intellij's license feature, it automatically updates the license text of modified files even for all file formats if required.
--
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
Re: [PR] HDDS-13794. Use synchronized logger for auditing tasks [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on code in PR #9229:
URL: https://github.com/apache/ozone/pull/9229#discussion_r2482995671
##########
hadoop-ozone/dist/src/shell/ozone/ozone:
##########
@@ -107,7 +109,7 @@ function ozonecmd_case
ozone_deprecate_envvar HDDS_DN_OPTS OZONE_DATANODE_OPTS
OZONE_DATANODE_OPTS="${RATIS_OPTS} ${OZONE_DATANODE_OPTS}"
OZONE_DATANODE_OPTS="-Dlog4j.configurationFile=${OZONE_CONF_DIR}/dn-audit-log4j2.properties,${OZONE_CONF_DIR}/dn-container-log4j2.properties ${OZONE_DATANODE_OPTS}"
- OZONE_DATANODE_OPTS="-Dlog4j2.contextSelector=org.apache.logging.log4j.core.async.AsyncLoggerContextSelector ${OZONE_DATANODE_OPTS} ${OZONE_MODULE_ACCESS_ARGS}"
+ OZONE_DATANODE_OPTS="${OZONE_DATANODE_OPTS} ${OZONE_MODULE_ACCESS_ARGS}"
Review Comment:
Yes, this is already happening.
Log4Jv2's AsyncLogger was used only for auditing. This PR removes this.
Log4Jv1's AsyncAppender is used for all regular logging including dependencies and libraries.
--
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
Re: [PR] HDDS-13794: Use synchronized logger for auditing tasks [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9229:
URL: https://github.com/apache/ozone/pull/9229#discussion_r2482465741
##########
hadoop-ozone/s3gateway/src/test/resources/auditlog.properties:
##########
@@ -1,18 +1,18 @@
#
# 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>
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
Review Comment:
Any reason why we need to change the license headers?
##########
hadoop-ozone/dist/src/shell/ozone/ozone:
##########
@@ -107,7 +109,7 @@ function ozonecmd_case
ozone_deprecate_envvar HDDS_DN_OPTS OZONE_DATANODE_OPTS
OZONE_DATANODE_OPTS="${RATIS_OPTS} ${OZONE_DATANODE_OPTS}"
OZONE_DATANODE_OPTS="-Dlog4j.configurationFile=${OZONE_CONF_DIR}/dn-audit-log4j2.properties,${OZONE_CONF_DIR}/dn-container-log4j2.properties ${OZONE_DATANODE_OPTS}"
- OZONE_DATANODE_OPTS="-Dlog4j2.contextSelector=org.apache.logging.log4j.core.async.AsyncLoggerContextSelector ${OZONE_DATANODE_OPTS} ${OZONE_MODULE_ACCESS_ARGS}"
+ OZONE_DATANODE_OPTS="${OZONE_DATANODE_OPTS} ${OZONE_MODULE_ACCESS_ARGS}"
Review Comment:
is synchronized default? Do you think we can use asynchornized logger for the jvm logs and use sync logger only for audit logs? Is it possible to separate this out based on the log4j properties?
--
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
Re: [PR] HDDS-13794. Use synchronized logger for auditing tasks [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 merged PR #9229:
URL: https://github.com/apache/ozone/pull/9229
--
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
Re: [PR] HDDS-13794. Use synchronized logger for auditing tasks [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on PR #9229:
URL: https://github.com/apache/ozone/pull/9229#issuecomment-3505178948
Thanks for the review @ivandika3.
No significant performance difference was observed between the async and sync logging configurations.
Please see the performance numbers in https://github.com/apache/ozone/pull/9264. The rendered document can be seen [here](https://github.com/ptlrs/ozone/blob/0886ea3db354682fa26447c5bffbdafd4f11aadb/hadoop-hdds/docs/content/design/logger-benchmark.md).
--
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
Re: [PR] HDDS-13794. Use synchronized logger for auditing tasks [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on code in PR #9229:
URL: https://github.com/apache/ozone/pull/9229#discussion_r2482999620
##########
hadoop-ozone/s3gateway/src/test/resources/auditlog.properties:
##########
@@ -1,18 +1,18 @@
#
# 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>
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
Review Comment:
https://issues.apache.org/jira/browse/HDDS-12251 updated the license format but it appears that the new license text was updated only for Java files.
Since I use Intellij's license feature, it automatically updates the license text of modified files for all file formats if required.
--
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
Re: [PR] HDDS-13794. Use synchronized logger for auditing tasks [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on code in PR #9229:
URL: https://github.com/apache/ozone/pull/9229#discussion_r2482999620
##########
hadoop-ozone/s3gateway/src/test/resources/auditlog.properties:
##########
@@ -1,18 +1,18 @@
#
# 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>
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
Review Comment:
https://issues.apache.org/jira/browse/HDDS-12251 updated the license format but it appears that the new license text was updated only for Java files.
As I use Intellij's license feature, it automatically updates the license text of modified files for all file formats if required.
--
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
Re: [PR] HDDS-13794. Use synchronized logger for auditing tasks [ozone]
Posted by "ptlrs (via GitHub)" <gi...@apache.org>.
ptlrs commented on code in PR #9229:
URL: https://github.com/apache/ozone/pull/9229#discussion_r2482995671
##########
hadoop-ozone/dist/src/shell/ozone/ozone:
##########
@@ -107,7 +109,7 @@ function ozonecmd_case
ozone_deprecate_envvar HDDS_DN_OPTS OZONE_DATANODE_OPTS
OZONE_DATANODE_OPTS="${RATIS_OPTS} ${OZONE_DATANODE_OPTS}"
OZONE_DATANODE_OPTS="-Dlog4j.configurationFile=${OZONE_CONF_DIR}/dn-audit-log4j2.properties,${OZONE_CONF_DIR}/dn-container-log4j2.properties ${OZONE_DATANODE_OPTS}"
- OZONE_DATANODE_OPTS="-Dlog4j2.contextSelector=org.apache.logging.log4j.core.async.AsyncLoggerContextSelector ${OZONE_DATANODE_OPTS} ${OZONE_MODULE_ACCESS_ARGS}"
+ OZONE_DATANODE_OPTS="${OZONE_DATANODE_OPTS} ${OZONE_MODULE_ACCESS_ARGS}"
Review Comment:
Yes, this is already happening.
Log4Jv2 was used only for auditing. This PR removes the AsyncLogger.
Log4Jv1's AsyncAppender is used for all regular logging including dependencies and libraries.
--
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