Migration to log4j2

[snleee]

As stated in #3913, log4j has been identified as a
performance bottleneck for high qps use cases because
it is using a global lock for "Category.callAppenders()"
method.

log4j2 supports async logging so this migration should
improve the performance for high qps use cases.

[codecov-io]

Codecov Report

Merging #4139 into master will decrease coverage by 0.13%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #4139      +/-   ##
============================================
- Coverage     67.09%   66.95%   -0.14%     
  Complexity       20       20              
============================================
  Files          1037     1039       +2     
  Lines         51447    51818     +371     
  Branches       7200     7264      +64     
============================================
+ Hits          34517    34696     +179     
- Misses        14567    14720     +153     
- Partials       2363     2402      +39

Impacted Files	Coverage Δ	Complexity Δ
.../java/org/apache/pinot/common/utils/ZkStarter.java	72.05% <100%> (+0.41%)	0 <0> (ø)	⬇️
...a/manager/realtime/RealtimeSegmentDataManager.java	50% <0%> (-50%)	0% <0%> (ø)
...egation/function/customobject/MinMaxRangePair.java	75.86% <0%> (-24.14%)	0% <0%> (ø)
.../helix/core/realtime/SegmentCompletionManager.java	56.09% <0%> (-13.84%)	0% <0%> (ø)
...e/impl/dictionary/LongOnHeapMutableDictionary.java	82.22% <0%> (-13.34%)	0% <0%> (ø)
.../impl/dictionary/FloatOnHeapMutableDictionary.java	80% <0%> (-13.34%)	0% <0%> (ø)
...impl/dictionary/DoubleOnHeapMutableDictionary.java	68.88% <0%> (-13.34%)	0% <0%> (ø)
...e/operator/dociditerators/SortedDocIdIterator.java	55.55% <0%> (-8.34%)	0% <0%> (ø)
...impl/dictionary/FloatOffHeapMutableDictionary.java	81.81% <0%> (-5.46%)	0% <0%> (ø)
...lix/core/realtime/PinotRealtimeSegmentManager.java	73% <0%> (-2%)	0% <0%> (ø)
... and 26 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0086590...e8892bf. Read the comment docs.

[sunithabeeram]

Thanks for picking this up. Did you manually convert the existing log4j properties files to the corresponding xml versions (or was there an automated way to do it)? It will be good to also verify that this version indeed fixes the perf issues.

[snleee]

@sunithabeeram I manually converted the configs and they are slightly different e.g. [%x] -> [%t] https://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/PatternLayout.html

I will do testing before merging this.

[snleee]

For internal high qps use case, we see the following performance bump

1 broker, 1 server
dataset: 84GB
testing qps: 1k

with log4j
99.9% scheduler wait time: 17.56 avg, 176.9 max
99% broker side end-to-end latency: 78.17 avg, 258 max

with log4j2
99.9% scheduler wait time: 1.048 avg, 3.395 max
99% broker side end-to-end latency: 32.3 avg, 39.15 max

[sunithabeeram]

Nice work!

[sunithabeeram]

Do later versions of ZK work better with log4j2? Wondering what our plan should be to stop relying on log4j-1.2-api.

[snleee]

I think that we should still keep log4j-1.2-api since some of our dependencies are directly using log4j 1.x api (e.g. zookeeper). Zookeeper is still on log4j 1.x so using later version won't help here. There was some effort to do migration but the pr didn't get merged and closed. apache/zookeeper#148

[sunithabeeram]

Does this need a licensing header?

[snleee]

We ignore license check for test/resources but i will add the header for completeness.