Skip to content
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.

Sign up for GitHub

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

HIVE-28417: Bump Log4j2 to 2.24.1 to facilitate compilation of GraalVM Native Image #5375

Merged
merged 1 commit into from
Oct 25, 2024
Merged

HIVE-28417: Bump Log4j2 to 2.24.1 to facilitate compilation of GraalVM Native Image #5375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions data/conf/hive-log4j2.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

status = INFO
zhangbutao marked this conversation as resolved.
Show resolved Hide resolved
name = HiveLog4j2Test
packages = org.apache.hadoop.hive.ql.log
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

2024-08-02T10:55:16.634Z main WARN The use of package scanning to locate plugins is deprecated and will be removed in a future release
2024-08-02T10:55:16.650Z main WARN The use of package scanning to locate plugins is deprecated and will be removed in a future release
2024-08-02T10:55:16.670Z main WARN The use of package scanning to locate plugins is deprecated and will be removed in a future release
2024-08-02T10:55:16.686Z main WARN The use of package scanning to locate plugins is deprecated and will be removed in a future release

Copy link
Contributor

Choose a reason for hiding this comment

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

https://web.archive.org/web/20240823014436/https://logging.apache.org/log4j/2.x/manual/configuration.html#configuration-syntax

Use of the packages attribute is deprecated and will be removed in Log4j 3.0. Plugins should be processed with the Log4j annotation processor. A comma separated list of package names to search for plugins. Plugins are only loaded once per classloader so changing this value may not have any effect upon reconfiguration.

How we replace packages with the new way? I think removing package will cause some unkown issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

How we replace packages with the new way? I think removing package will cause some unkown issue.

  • I would say that even without merging this PR, packages never worked in Hive. Unless Hive has always used log4j1 and never used log4j2. Because the class org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor exists in the JAR of the final binary of Hive, org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor generates Log4j2Plugins.dat, which directly covers the operation of packages. It is located at /hive/ql/target/hive-exec-4.1.0-SNAPSHOT.jar!/META-INF/org/apache/logging/log4j/core/config/plugins/Log4j2Plugins.dat. packages is actually a log4j1 thing.
  • image
  • You can determine from a comment by a log4j2 committer that this property does not actually take effect. See Fix handling of onMatch and onMismatch attributes in the properties configuration format logging-log4j2#2791 (comment) .

There is no problem in Apache Hive: I checked that the appropriate annotation processor is configured in the hive-exec project and there is a Log4j2Plugins.dat file in the JAR. You can safely remove the packages configuration attribute.

<plugin>
  <groupId>org.apache.maven.plugins</groupId>
  <artifactId>maven-compiler-plugin</artifactId>
  <version>${maven-compiler-plugin.version}</version>
  <executions>
    <execution>
      <id>generate-log4j-plugin-descriptor</id>
      <goals>
        <goal>compile</goal>
      </goals>
      <phase>process-classes</phase>
      <configuration>
        <proc>only</proc>
        <annotationProcessorPaths>
          <path>
            <groupId>org.apache.logging.log4j</groupId>
            <artifactId>log4j-core</artifactId>
            <version>2.24.1</version>
          </path>
        </annotationProcessorPaths>
        <annotationProcessors>
          <processor>org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor</processor>
        </annotationProcessors>
      </configuration>
    </execution>
  </executions>
</plugin>

Copy link
Contributor

Choose a reason for hiding this comment

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

Good explanation!

Since Hive has neither CI for JDK23 and JDK24 nor CI for GraalVM Native Image, we actually don't need to register a org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor in pom.xml as https://logging.apache.org/log4j/2.x/manual/plugins.html#plugin-registry .

BTW, if Hive wants to use JDK23 or JDK24, should we need to register the PluginProcessor ?

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW, if Hive wants to use JDK23 or JDK24, should we need to register the PluginProcessor ?

  • The answer is yes. But I think most people will actually choose to set -proc:full for JDK 23 to make JDK 23 behave the same as JDK8. -proc:full is only available on 11.0.23, 17.0.11, JDK 21 - 24. -proc:full is not available on any OpenJDK 8u, -proc:full is only available on Oracle's closed source JDK 8u411.
  • For JDK23, we can set <maven.compiler.proc>full</maven.compiler.proc> in a separate Maven Profile to make JDK23 behave the same as JDK 8. Refer to Support building Example module with OpenJDK 23 shardingsphere#33224 for the solution.
  • I would say setting <maven.compiler.proc>full</maven.compiler.proc> is not useful for Hive, as I found that the master branch of Hive cannot be compiled with JDK22. This means that none of the unit tests will be affected.


# list of properties
property.hive.log.level = DEBUG
Expand All @@ -25,9 +23,6 @@ property.hive.log.dir = ${sys:test.tmp.dir}/log
property.hive.log.file = hive.log
property.hive.test.console.log.level = INFO

# list of all appenders
appenders = console, DRFA
zhangbutao marked this conversation as resolved.
Show resolved Hide resolved

# console appender
appender.console.type = Console
appender.console.name = console
Expand All @@ -49,9 +44,6 @@ appender.DRFA.policies.time.modulate = true
appender.DRFA.strategy.type = DefaultRolloverStrategy
appender.DRFA.strategy.max = 30

# list of all loggers
loggers = HadoopIPC, HadoopSecurity, Hdfs, HdfsServer, HadoopMetrics2, Mortbay, Yarn, YarnServer, Tez, HadoopConf, Zookeeper, ServerCnxn, NIOServerCnxn, ClientCnxn, ClientCnxnSocket, ClientCnxnSocketNIO, DataNucleus, Datastore, JPOX, Operator, Serde2Lazy, ObjectStore, CalcitePlanner, AmazonAws, ApacheHttp, Thrift, Jetty, BlockStateChange, swo, CBORuleLogger
linghengqian marked this conversation as resolved.
Show resolved Hide resolved

logger.HadoopIPC.name = org.apache.hadoop.ipc
logger.HadoopIPC.level = WARN

Expand Down Expand Up @@ -128,7 +120,7 @@ logger.CBORuleLogger.filter.marker.type = MarkerFilter
logger.CBORuleLogger.filter.marker.marker = FULL_PLAN
# Change filter to ACCEPT, to see the produced plan after every rule invocation using the EXPLAIN CBO format
logger.CBORuleLogger.filter.marker.onMatch = DENY
logger.CBORuleLogger.filter.marker.onMisMatch = NEUTRAL
logger.CBORuleLogger.filter.marker.onMismatch = NEUTRAL

logger.AmazonAws.name=com.amazonaws
logger.AmazonAws.level = INFO
Expand Down
3 changes: 1 addition & 2 deletions llap-server/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,9 +367,8 @@
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
<artifactId>log4j-core-test</artifactId>
<version>${log4j2.version}</version>
<classifier>tests</classifier>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
5 changes: 3 additions & 2 deletions llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@
import org.apache.tez.common.security.TokenCache;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.Marker;
import org.slf4j.MDC;
import org.slf4j.Marker;
import org.slf4j.impl.StaticMarkerBinder;

import java.io.IOException;
import java.util.Collections;
Expand All @@ -70,7 +71,7 @@ public class QueryTracker extends AbstractService {

private static final Logger LOG = LoggerFactory.getLogger(QueryTracker.class);
private static final Marker QUERY_COMPLETE_MARKER =
new Log4jMarker(new Log4jQueryCompleteMarker());
new Log4jMarker(StaticMarkerBinder.getSingleton().getMarkerFactory(), new Log4jQueryCompleteMarker());
zhangbutao marked this conversation as resolved.
Show resolved Hide resolved

/// Shared singleton MetricsSource instance for all DAG locks
private static final MetricsSource LOCK_METRICS;
Expand Down
2 changes: 1 addition & 1 deletion llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TestLlapDaemonLogging.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.apache.hadoop.security.token.Token;
import org.apache.hive.testutils.junit.extensions.DoNothingTCPServer;
import org.apache.hive.testutils.junit.extensions.DoNothingTCPServerExtension;
import org.apache.logging.log4j.junit.LoggerContextSource;
import org.apache.logging.log4j.core.test.junit.LoggerContextSource;
import org.apache.tez.common.security.TokenCache;

import org.junit.jupiter.api.Test;
Expand Down
3 changes: 1 addition & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@
<!-- Leaving libfb303 at 0.9.3 regardless of libthrift: As per THRIFT-4613 The Apache Thrift project does not publish items related to fb303 at this point -->
<libfb303.version>0.9.3</libfb303.version>
<libthrift.version>0.16.0</libthrift.version>
<log4j2.version>2.18.0</log4j2.version>
<log4j2.version>2.24.1</log4j2.version>
<mariadb.version>2.5.0</mariadb.version>
<mssql.version>6.2.1.jre8</mssql.version>
<mysql.version>8.0.31</mysql.version>
Expand Down Expand Up @@ -1791,7 +1791,6 @@
<log4j.configurationFile>${test.log4j.scheme}${test.conf.dir}/hive-log4j2.properties</log4j.configurationFile>
<hive.test.console.log.level>${test.console.log.level}</hive.test.console.log.level>
<hive.cluster.id>hive-test-cluster-id-cli</hive.cluster.id>
<log4j.debug>true</log4j.debug>
zhangbutao marked this conversation as resolved.
Show resolved Hide resolved
<!-- don't dirty up /tmp -->
<java.io.tmpdir>${test.tmp.dir}</java.io.tmpdir>
<!-- Hadoop's minidfs class uses this -->
Expand Down
8 changes: 6 additions & 2 deletions standalone-metastore/metastore-server/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,13 @@
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
<artifactId>log4j-core-test</artifactId>
<version>${log4j2.version}</version>
<classifier>tests</classifier>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
Expand Down
2 changes: 1 addition & 1 deletion ...server/src/test/java/org/apache/hadoop/hive/metastore/utils/TestMetaStoreServerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.LoggerContext;
import org.apache.logging.log4j.core.config.LoggerConfig;
import org.apache.logging.log4j.test.appender.ListAppender;
import org.apache.logging.log4j.core.test.appender.ListAppender;
import org.apache.thrift.TException;
import org.hamcrest.core.IsNot;
import org.junit.After;
Expand Down
2 changes: 1 addition & 1 deletion standalone-metastore/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
<junit.vintage.version>5.6.3</junit.vintage.version>
<libfb303.version>0.9.3</libfb303.version>
<libthrift.version>0.16.0</libthrift.version>
<log4j2.version>2.18.0</log4j2.version>
<log4j2.version>2.24.1</log4j2.version>
<mockito-core.version>3.4.4</mockito-core.version>
<orc.version>1.9.4</orc.version>
<protobuf.version>3.24.4</protobuf.version>
Expand Down
Loading