Skip to content

HIVE-29026: Cleanup Log4j properties file to adhere to 2.x changes #5885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Aggarwal-Raghav
Copy link
Contributor

What changes were proposed in this pull request?

Cleanup log4j 1.x syntax. Majorly the changes are same as of HIVE-28417

Why are the changes needed?

For log4j 2.24.3, the changes to remove packages attribute due to LOG4J2-3644 was done only in data/conf/hive-log4j2.properties.
but
packaging/target/apache-hive-4.1.0-SNAPSHOT-bin/apache-hive-4.1.0-SNAPSHOT-bin/conf/hive-log4j2.properties.template is picked from common/src/main/resources/hive-log4j2.properties which cause the following WARN message.

2025-06-16T20:51:52.848390Z main WARN The use of package scanning to locate Log4j plugins is deprecated.
Please remove the `packages` attribute from your configuration file.
See https://logging.apache.org/log4j/2.x/faq.html#package-scanning for details. 

Does this PR introduce any user-facing change?

NO

How was this patch tested?

Will see CI output

@Aggarwal-Raghav
Copy link
Contributor Author

Have moved datanucleus log4j from 1.x to 2.x based on https://www.datanucleus.org/products/accessplatform_6_0/logging.html

Removed iceberg/iceberg-handler/src/test/resources/log4j.properties, as it is says "Configuration for modules which are using log4j v1 (e.g. Hive2, DataNucleus)" If there are any UT failures then will migrate it to 2.x

Copy link

@Aggarwal-Raghav Aggarwal-Raghav changed the title [WIP] HIVE-29026: Cleanup Log4j properties file to adhere to 2.x changes HIVE-29026: Cleanup Log4j properties file to adhere to 2.x changes Jun 21, 2025
@@ -14,7 +14,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

status = FATAL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I've not checked how Log4j 2 works, I am feeling that the original intention to see only FATAL will be gone

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @okumin. Very valid question, when I was working on the PR, i also though about it.
Based on the comments apache/logging-log4j2#2794 (comment)

Status Logger is intended for troubleshooting Log4j itself and its components. I strongly advise you to avoid configuring it unless you are troubleshooting a Log4j issue.

Because of that I went ahead and removed. I don't have any strong opinion on this, If you think its better to keep, then I can update the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants