You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2019/05/10 16:51:53 UTC

[kudu-CR] [java] Upgrade to log4j 2 

Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13304


Change subject: [java] Upgrade to log4j 2
......................................................................

[java] Upgrade to log4j 2

This patch upgrades from log4j 1.2.17 to 2.11.2 and
includes any neccessary changes to support the
upgrade.

Log4j 2.x has been out for 5 years now and log4j1.x has
been marked as EOL as of August 2015.

The changes required to support Log4j 2.x are test only
because we use Slf4j to abstract away our concrete
logger implementation and allow users to use any
logging implementaton they like.

Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
---
M java/gradle/dependencies.gradle
M java/kudu-backup/build.gradle
R java/kudu-backup/src/test/resources/log4j2.properties
M java/kudu-client-tools/build.gradle
R java/kudu-client-tools/src/test/resources/log4j2.properties
M java/kudu-client/build.gradle
R java/kudu-client/src/test/resources/log4j2.properties
M java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerParseErrorTest.java
R java/kudu-flume-sink/src/test/resources/log4j2.properties
M java/kudu-hive/build.gradle
R java/kudu-hive/src/test/resources/log4j2.properties
M java/kudu-mapreduce/build.gradle
R java/kudu-mapreduce/src/test/resources/log4j2.properties
M java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/resources/log4j2.properties
M java/kudu-spark/build.gradle
R java/kudu-spark/src/test/resources/log4j2.properties
M java/kudu-test-utils/build.gradle
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
R java/kudu-test-utils/src/test/resources/log4j2.properties
22 files changed, 189 insertions(+), 102 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13304/1
-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13304

to look at the new patch set (#5).

Change subject: [java] Upgrade to log4j 2
......................................................................

[java] Upgrade to log4j 2

This patch upgrades from log4j 1.2.17 to 2.11.2 and
includes any neccessary changes to support the
upgrade.

Log4j 2.x has been out for 5 years now and log4j1.x has
been marked as EOL as of August 2015.

The changes required to support Log4j 2.x are test only
because we use Slf4j to abstract away our concrete
logger implementation and allow users to use any
logging implementaton they like.

Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
---
M java/gradle/dependencies.gradle
M java/kudu-backup-tools/build.gradle
R java/kudu-backup-tools/src/test/resources/log4j2.properties
M java/kudu-backup/build.gradle
R java/kudu-backup/src/test/resources/log4j2.properties
M java/kudu-client-tools/build.gradle
R java/kudu-client-tools/src/test/resources/log4j2.properties
M java/kudu-client/build.gradle
R java/kudu-client/src/test/resources/log4j2.properties
M java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerParseErrorTest.java
R java/kudu-flume-sink/src/test/resources/log4j2.properties
M java/kudu-hive/build.gradle
R java/kudu-hive/src/test/resources/log4j2.properties
M java/kudu-mapreduce/build.gradle
R java/kudu-mapreduce/src/test/resources/log4j2.properties
M java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/resources/log4j2.properties
M java/kudu-spark/build.gradle
R java/kudu-spark/src/test/resources/log4j2.properties
M java/kudu-test-utils/build.gradle
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
R java/kudu-test-utils/src/test/resources/log4j2.properties
24 files changed, 219 insertions(+), 122 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13304/5
-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................


Patch Set 4: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 May 2019 17:53:32 +0000
Gerrit-HasComments: No

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-backup/src/test/resources/log4j2.properties
File java/kudu-backup/src/test/resources/log4j2.properties:

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-backup/src/test/resources/log4j2.properties@19
PS1, Line 19: name = PropertiesConfig
> Nit:
Done


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-backup/src/test/resources/log4j2.properties@25
PS1, Line 25: %
> Why is there an isolated open bracket here?
Done


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java:

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java@42
PS1, Line 42: 
> Why is this layout in the capturing appender though? It's not logging that 
Done


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java@50
PS1, Line 50:     appended.append(LAYOUT.toSerializable(event));
> Can you annotate the meaning of null and true?
Done


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java:

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@74
PS1, Line 74:     try (OutputStream os = createOutputStream(useGzip)) {
> Annotate null and true.
Done


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@99
PS1, Line 99:         // ignored
            :       }
            :       outputFileWriter = null;
            :     }
            :     if 
> Isn't this the same as IOUtils.closeQuietly?
Yes it is, but IOUtils.closeQuietly is deprecated and the deprecated message indicates to use this style. 

https://commons.apache.org/proper/commons-io/javadocs/api-release/org/apache/commons/io/IOUtils.html#closeQuietly-java.io.Closeable...-



-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 13 May 2019 18:02:55 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13304/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java:

http://gerrit.cloudera.org:8080/#/c/13304/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@77
PS2, Line 77:       // BufferedWriter to buffer up character conversions.
> I guess there's also an issue with the old code if the BufferedWriter const
The BufferedWriter does throw any checked exceptions.


http://gerrit.cloudera.org:8080/#/c/13304/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@87
PS2, Line 87:     if (useGzip) {
> If this throws, where do we close 'os' (i.e. the FileOutputStream created o
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 14 May 2019 12:20:57 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 14 May 2019 21:06:24 +0000
Gerrit-HasComments: No

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................


Patch Set 1: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 10 May 2019 17:25:02 +0000
Gerrit-HasComments: No

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13304

to look at the new patch set (#4).

Change subject: [java] Upgrade to log4j 2
......................................................................

[java] Upgrade to log4j 2

This patch upgrades from log4j 1.2.17 to 2.11.2 and
includes any neccessary changes to support the
upgrade.

Log4j 2.x has been out for 5 years now and log4j1.x has
been marked as EOL as of August 2015.

The changes required to support Log4j 2.x are test only
because we use Slf4j to abstract away our concrete
logger implementation and allow users to use any
logging implementaton they like.

Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
---
M java/gradle/dependencies.gradle
M java/kudu-backup/build.gradle
R java/kudu-backup/src/test/resources/log4j2.properties
M java/kudu-client-tools/build.gradle
R java/kudu-client-tools/src/test/resources/log4j2.properties
M java/kudu-client/build.gradle
R java/kudu-client/src/test/resources/log4j2.properties
M java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerParseErrorTest.java
R java/kudu-flume-sink/src/test/resources/log4j2.properties
M java/kudu-hive/build.gradle
R java/kudu-hive/src/test/resources/log4j2.properties
M java/kudu-mapreduce/build.gradle
R java/kudu-mapreduce/src/test/resources/log4j2.properties
M java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/resources/log4j2.properties
M java/kudu-spark/build.gradle
R java/kudu-spark/src/test/resources/log4j2.properties
M java/kudu-test-utils/build.gradle
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
R java/kudu-test-utils/src/test/resources/log4j2.properties
22 files changed, 204 insertions(+), 116 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13304/4
-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13304

to look at the new patch set (#2).

Change subject: [java] Upgrade to log4j 2
......................................................................

[java] Upgrade to log4j 2

This patch upgrades from log4j 1.2.17 to 2.11.2 and
includes any neccessary changes to support the
upgrade.

Log4j 2.x has been out for 5 years now and log4j1.x has
been marked as EOL as of August 2015.

The changes required to support Log4j 2.x are test only
because we use Slf4j to abstract away our concrete
logger implementation and allow users to use any
logging implementaton they like.

Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
---
M java/gradle/dependencies.gradle
M java/kudu-backup/build.gradle
R java/kudu-backup/src/test/resources/log4j2.properties
M java/kudu-client-tools/build.gradle
R java/kudu-client-tools/src/test/resources/log4j2.properties
M java/kudu-client/build.gradle
R java/kudu-client/src/test/resources/log4j2.properties
M java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerParseErrorTest.java
R java/kudu-flume-sink/src/test/resources/log4j2.properties
M java/kudu-hive/build.gradle
R java/kudu-hive/src/test/resources/log4j2.properties
M java/kudu-mapreduce/build.gradle
R java/kudu-mapreduce/src/test/resources/log4j2.properties
M java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/resources/log4j2.properties
M java/kudu-spark/build.gradle
R java/kudu-spark/src/test/resources/log4j2.properties
M java/kudu-test-utils/build.gradle
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
R java/kudu-test-utils/src/test/resources/log4j2.properties
22 files changed, 195 insertions(+), 116 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13304/2
-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13304/4/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java:

http://gerrit.cloudera.org:8080/#/c/13304/4/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java@40
PS4, Line 40:   private static final PatternLayout LAYOUT = PatternLayout.newBuilder()
It turns out MessageLayout doesn't work like I expected. It doesn't substitute logging {} arguments. I changed this to use the same PatternLayout as the file appender. I think this makes sense given this is the same layout we define in the test log4j properties files.



-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 May 2019 13:40:28 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13304

to look at the new patch set (#3).

Change subject: [java] Upgrade to log4j 2
......................................................................

[java] Upgrade to log4j 2

This patch upgrades from log4j 1.2.17 to 2.11.2 and
includes any neccessary changes to support the
upgrade.

Log4j 2.x has been out for 5 years now and log4j1.x has
been marked as EOL as of August 2015.

The changes required to support Log4j 2.x are test only
because we use Slf4j to abstract away our concrete
logger implementation and allow users to use any
logging implementaton they like.

Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
---
M java/gradle/dependencies.gradle
M java/kudu-backup/build.gradle
R java/kudu-backup/src/test/resources/log4j2.properties
M java/kudu-client-tools/build.gradle
R java/kudu-client-tools/src/test/resources/log4j2.properties
M java/kudu-client/build.gradle
R java/kudu-client/src/test/resources/log4j2.properties
M java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerParseErrorTest.java
R java/kudu-flume-sink/src/test/resources/log4j2.properties
M java/kudu-hive/build.gradle
R java/kudu-hive/src/test/resources/log4j2.properties
M java/kudu-mapreduce/build.gradle
R java/kudu-mapreduce/src/test/resources/log4j2.properties
M java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/resources/log4j2.properties
M java/kudu-spark/build.gradle
R java/kudu-spark/src/test/resources/log4j2.properties
M java/kudu-test-utils/build.gradle
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
R java/kudu-test-utils/src/test/resources/log4j2.properties
22 files changed, 201 insertions(+), 116 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/13304/3
-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................

[java] Upgrade to log4j 2

This patch upgrades from log4j 1.2.17 to 2.11.2 and
includes any neccessary changes to support the
upgrade.

Log4j 2.x has been out for 5 years now and log4j1.x has
been marked as EOL as of August 2015.

The changes required to support Log4j 2.x are test only
because we use Slf4j to abstract away our concrete
logger implementation and allow users to use any
logging implementaton they like.

Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Reviewed-on: http://gerrit.cloudera.org:8080/13304
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M java/gradle/dependencies.gradle
M java/kudu-backup-tools/build.gradle
R java/kudu-backup-tools/src/test/resources/log4j2.properties
M java/kudu-backup/build.gradle
R java/kudu-backup/src/test/resources/log4j2.properties
M java/kudu-client-tools/build.gradle
R java/kudu-client-tools/src/test/resources/log4j2.properties
M java/kudu-client/build.gradle
R java/kudu-client/src/test/resources/log4j2.properties
M java/kudu-flume-sink/build.gradle
M java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/RegexpKuduOperationsProducerParseErrorTest.java
R java/kudu-flume-sink/src/test/resources/log4j2.properties
M java/kudu-hive/build.gradle
R java/kudu-hive/src/test/resources/log4j2.properties
M java/kudu-mapreduce/build.gradle
R java/kudu-mapreduce/src/test/resources/log4j2.properties
M java/kudu-spark-tools/build.gradle
R java/kudu-spark-tools/src/test/resources/log4j2.properties
M java/kudu-spark/build.gradle
R java/kudu-spark/src/test/resources/log4j2.properties
M java/kudu-test-utils/build.gradle
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
R java/kudu-test-utils/src/test/resources/log4j2.properties
24 files changed, 219 insertions(+), 122 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Adar Dembo: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java:

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@74
PS1, Line 74:     try (OutputStream os = createOutputStream(useGzip)) {
> Done
Missed this.


http://gerrit.cloudera.org:8080/#/c/13304/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java:

http://gerrit.cloudera.org:8080/#/c/13304/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@77
PS2, Line 77:       outputFileWriter = new BufferedWriter(new OutputStreamWriter(os, UTF_8));
I guess there's also an issue with the old code if the BufferedWriter constructor throws: who closes the OutputStreamWriter? Or is it sufficient to close 'os'?


http://gerrit.cloudera.org:8080/#/c/13304/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@87
PS2, Line 87:       os = new GZIPOutputStream(os);
If this throws, where do we close 'os' (i.e. the FileOutputStream created on L85)? That's what the old code was trying to do with the inner catch.

I don't think the new try-with-resources in the constructor does that, because if createOutputStream throws, 'os' will not have been assigned.



-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 13 May 2019 18:10:59 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-backup/src/test/resources/log4j2.properties
File java/kudu-backup/src/test/resources/log4j2.properties:

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-backup/src/test/resources/log4j2.properties@19
PS1, Line 19: name=PropertiesConfig
Nit:

  name = PropertiesConfig


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-backup/src/test/resources/log4j2.properties@25
PS1, Line 25: [
Why is there an isolated open bracket here?

It's in other log4j2.properties files too.


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java:

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java@42
PS1, Line 42:   // This is the standard layout used in Kudu tests.
Why is this layout in the capturing appender though? It's not logging that anyone is going to read.


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java@50
PS1, Line 50:     super("CapturingLogAppender", null, LAYOUT, true, Property.EMPTY_ARRAY);
Can you annotate the meaning of null and true?


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
File java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java:

http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@74
PS1, Line 74:     super("CapturingToFileLogAppender", null, LAYOUT, true, Property.EMPTY_ARRAY);
Annotate null and true.


http://gerrit.cloudera.org:8080/#/c/13304/1/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java@99
PS1, Line 99:       try {
            :         outputFileWriter.close();
            :       } catch (final IOException ioe) {
            :         // ignored
            :       }
Isn't this the same as IOUtils.closeQuietly?



-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 10 May 2019 21:31:19 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13304 )

Change subject: [java] Upgrade to log4j 2
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 16 May 2019 18:21:46 +0000
Gerrit-HasComments: No

[kudu-CR] [java] Upgrade to log4j 2

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.

Change subject: [java] Upgrade to log4j 2
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/13304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ic4e14a3a25c36d5f47d07f06c1c0e68dad42bc66
Gerrit-Change-Number: 13304
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)