You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ro...@apache.org on 2023/12/11 19:29:19 UTC

(pinot) branch master updated: PR to address Issue #12127 - Refactor foreign logger initialization in multiple classes (#12128)

This is an automated email from the ASF dual-hosted git repository.

rongr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new d68a5a8dac PR to address Issue #12127 - Refactor foreign logger initialization in multiple classes (#12128)
d68a5a8dac is described below

commit d68a5a8dac716433081d86b83691c97b4ae2b5af
Author: Tim Veil <32...@users.noreply.github.com>
AuthorDate: Mon Dec 11 14:29:12 2023 -0500

    PR to address Issue #12127 - Refactor foreign logger initialization in multiple classes (#12128)
    
    * Refactor logger initialization in multiple classes
    
    This commit addresses a widespread issue of Logger instances being incorrectly initialized in multiple classes. The Logger instances were mostly being created with the wrong class as argument to the LoggerFactory.getLogger() method. This issue has now been fixed by replacing the incorrect class names with the corresponding class in which the Logger instance is initialized. #12127
---
 .../src/main/java/org/apache/pinot/client/PinotConnection.java        | 4 ----
 .../apache/pinot/controller/recommender/rules/impl/JsonIndexRule.java | 2 +-
 .../main/java/org/apache/pinot/core/transport/DirectOOMHandler.java   | 2 +-
 .../java/org/apache/pinot/query/mailbox/InMemorySendingMailbox.java   | 2 +-
 .../main/java/org/apache/pinot/query/service/server/QueryServer.java  | 3 +--
 .../segment/local/recordtransformer/SpecialValueTransformer.java      | 2 +-
 .../local/segment/creator/impl/inv/text/LuceneFSTIndexCreator.java    | 3 +--
 .../local/segment/index/readers/vector/HnswVectorIndexReader.java     | 3 +--
 .../org/apache/pinot/segment/local/utils/ConsistentDataPushUtils.java | 2 +-
 .../java/org/apache/pinot/segment/local/utils/fst/RegexpMatcher.java  | 3 ---
 .../apache/pinot/tools/admin/command/ChangeNumReplicasCommand.java    | 2 +-
 .../java/org/apache/pinot/tools/streams/AvroFileSourceGenerator.java  | 2 +-
 12 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotConnection.java b/pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotConnection.java
index 9e703b5f3e..1e6a667ce7 100644
--- a/pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotConnection.java
+++ b/pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotConnection.java
@@ -18,7 +18,6 @@
  */
 package org.apache.pinot.client;
 
-import java.sql.Connection;
 import java.sql.DatabaseMetaData;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
@@ -33,13 +32,10 @@ import org.apache.pinot.client.controller.PinotControllerTransport;
 import org.apache.pinot.client.controller.PinotControllerTransportFactory;
 import org.apache.pinot.client.controller.response.ControllerTenantBrokerResponse;
 import org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 
 public class PinotConnection extends AbstractBaseConnection {
 
-  private static final Logger LOGGER = LoggerFactory.getLogger(Connection.class);
   protected static final String[] POSSIBLE_QUERY_OPTIONS = {
     QueryOptionKey.ENABLE_NULL_HANDLING,
     QueryOptionKey.USE_MULTISTAGE_ENGINE
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/impl/JsonIndexRule.java b/pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/impl/JsonIndexRule.java
index df36f742d7..5b97094b9c 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/impl/JsonIndexRule.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/recommender/rules/impl/JsonIndexRule.java
@@ -30,7 +30,7 @@ import org.slf4j.LoggerFactory;
 
 /** JSON columns must be NoDictionary columns with JsonIndex. */
 public class JsonIndexRule extends AbstractRule {
-  private static final Logger LOGGER = LoggerFactory.getLogger(RangeIndexRule.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(JsonIndexRule.class);
 
   public JsonIndexRule(InputManager input, ConfigManager output) {
     super(input, output);
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/transport/DirectOOMHandler.java b/pinot-core/src/main/java/org/apache/pinot/core/transport/DirectOOMHandler.java
index fad085a02e..dc79c0ba95 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/transport/DirectOOMHandler.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/transport/DirectOOMHandler.java
@@ -35,7 +35,7 @@ import org.slf4j.LoggerFactory;
  * no one will reach channelRead0.
  */
 public class DirectOOMHandler extends ChannelInboundHandlerAdapter {
-  private static final Logger LOGGER = LoggerFactory.getLogger(DataTableHandler.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(DirectOOMHandler.class);
   private static final AtomicBoolean DIRECT_OOM_SHUTTING_DOWN = new AtomicBoolean(false);
   private final QueryRouter _queryRouter;
   private final ServerRoutingInstance _serverRoutingInstance;
diff --git a/pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/InMemorySendingMailbox.java b/pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/InMemorySendingMailbox.java
index 306cd6992a..d1452712bc 100644
--- a/pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/InMemorySendingMailbox.java
+++ b/pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/InMemorySendingMailbox.java
@@ -27,7 +27,7 @@ import org.slf4j.LoggerFactory;
 
 
 public class InMemorySendingMailbox implements SendingMailbox {
-  private static final Logger LOGGER = LoggerFactory.getLogger(GrpcSendingMailbox.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(InMemorySendingMailbox.class);
 
   private final String _id;
   private final MailboxService _mailboxService;
diff --git a/pinot-query-runtime/src/main/java/org/apache/pinot/query/service/server/QueryServer.java b/pinot-query-runtime/src/main/java/org/apache/pinot/query/service/server/QueryServer.java
index 281f434d30..4a4daa148b 100644
--- a/pinot-query-runtime/src/main/java/org/apache/pinot/query/service/server/QueryServer.java
+++ b/pinot-query-runtime/src/main/java/org/apache/pinot/query/service/server/QueryServer.java
@@ -33,7 +33,6 @@ import org.apache.pinot.common.exception.QueryException;
 import org.apache.pinot.common.proto.PinotQueryWorkerGrpc;
 import org.apache.pinot.common.proto.Worker;
 import org.apache.pinot.common.utils.NamedThreadFactory;
-import org.apache.pinot.core.transport.grpc.GrpcQueryServer;
 import org.apache.pinot.query.runtime.QueryRunner;
 import org.apache.pinot.query.runtime.plan.DistributedStagePlan;
 import org.apache.pinot.query.runtime.plan.serde.QueryPlanSerDeUtils;
@@ -47,7 +46,7 @@ import org.slf4j.LoggerFactory;
  * {@link QueryServer} is the GRPC server that accepts query plan requests sent from {@link QueryDispatcher}.
  */
 public class QueryServer extends PinotQueryWorkerGrpc.PinotQueryWorkerImplBase {
-  private static final Logger LOGGER = LoggerFactory.getLogger(GrpcQueryServer.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(QueryServer.class);
   // TODO: Inbound messages can get quite large because we send the entire stage metadata map in each call.
   // See https://github.com/apache/pinot/issues/10331
   private static final int MAX_INBOUND_MESSAGE_SIZE = 64 * 1024 * 1024;
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/SpecialValueTransformer.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/SpecialValueTransformer.java
index 484cae888a..d019384ed7 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/SpecialValueTransformer.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/SpecialValueTransformer.java
@@ -42,7 +42,7 @@ import org.slf4j.LoggerFactory;
  */
 public class SpecialValueTransformer implements RecordTransformer {
 
-  private static final Logger LOGGER = LoggerFactory.getLogger(NullValueTransformer.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(SpecialValueTransformer.class);
   private final HashSet<String> _specialValuesKeySet = new HashSet<>();
   private int _negativeZeroConversionCount = 0;
   private int _nanConversionCount = 0;
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/text/LuceneFSTIndexCreator.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/text/LuceneFSTIndexCreator.java
index 335449fe96..60b903739b 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/text/LuceneFSTIndexCreator.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/text/LuceneFSTIndexCreator.java
@@ -23,7 +23,6 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import org.apache.lucene.store.OutputStreamDataOutput;
 import org.apache.lucene.util.fst.FST;
-import org.apache.pinot.segment.local.segment.creator.impl.SegmentColumnarIndexCreator;
 import org.apache.pinot.segment.local.utils.fst.FSTBuilder;
 import org.apache.pinot.segment.spi.V1Constants;
 import org.apache.pinot.segment.spi.creator.IndexCreationContext;
@@ -39,7 +38,7 @@ import org.slf4j.LoggerFactory;
  *
  */
 public class LuceneFSTIndexCreator implements FSTIndexCreator {
-  private static final Logger LOGGER = LoggerFactory.getLogger(SegmentColumnarIndexCreator.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(LuceneFSTIndexCreator.class);
   private final File _fstIndexFile;
   private final FSTBuilder _fstBuilder;
   Integer _dictId;
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/vector/HnswVectorIndexReader.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/vector/HnswVectorIndexReader.java
index 079a4a5ce2..13b17365b0 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/vector/HnswVectorIndexReader.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/vector/HnswVectorIndexReader.java
@@ -33,7 +33,6 @@ import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.FSDirectory;
 import org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader;
 import org.apache.pinot.segment.local.segment.creator.impl.vector.HnswVectorIndexCreator;
-import org.apache.pinot.segment.local.segment.index.readers.text.LuceneTextIndexReader;
 import org.apache.pinot.segment.spi.V1Constants;
 import org.apache.pinot.segment.spi.index.creator.VectorIndexConfig;
 import org.apache.pinot.segment.spi.index.reader.VectorIndexReader;
@@ -45,7 +44,7 @@ import org.slf4j.LoggerFactory;
 
 public class HnswVectorIndexReader implements VectorIndexReader {
 
-  private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(LuceneTextIndexReader.class);
+  private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(HnswVectorIndexReader.class);
 
   private final IndexReader _indexReader;
   private final Directory _indexDirectory;
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/ConsistentDataPushUtils.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/ConsistentDataPushUtils.java
index 7296cc7928..9edcf9a718 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/ConsistentDataPushUtils.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/ConsistentDataPushUtils.java
@@ -50,7 +50,7 @@ public class ConsistentDataPushUtils {
   private ConsistentDataPushUtils() {
   }
 
-  private static final Logger LOGGER = LoggerFactory.getLogger(SegmentPushUtils.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(ConsistentDataPushUtils.class);
   private static final FileUploadDownloadClient FILE_UPLOAD_DOWNLOAD_CLIENT = new FileUploadDownloadClient();
   private static final RetryPolicy DEFAULT_RETRY_POLICY = RetryPolicies.exponentialBackoffRetryPolicy(5, 10_000L, 2.0);
   public static final String SEGMENT_NAME_POSTFIX = "segment.name.postfix";
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/fst/RegexpMatcher.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/fst/RegexpMatcher.java
index f761c2b8dd..409ecd3789 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/fst/RegexpMatcher.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/fst/RegexpMatcher.java
@@ -29,8 +29,6 @@ import org.apache.lucene.util.automaton.RegExp;
 import org.apache.lucene.util.automaton.Transition;
 import org.apache.lucene.util.fst.FST;
 import org.apache.lucene.util.fst.Util;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 
 /**
@@ -42,7 +40,6 @@ import org.slf4j.LoggerFactory;
  *   match(input) Function builds the automaton and matches given input.
  */
 public class RegexpMatcher {
-  public static final Logger LOGGER = LoggerFactory.getLogger(FSTBuilder.class);
 
   private final String _regexQuery;
   private final FST<Long> _fst;
diff --git a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/ChangeNumReplicasCommand.java b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/ChangeNumReplicasCommand.java
index e42039281a..c173edc2f5 100644
--- a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/ChangeNumReplicasCommand.java
+++ b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/ChangeNumReplicasCommand.java
@@ -27,7 +27,7 @@ import picocli.CommandLine;
 
 @CommandLine.Command(name = "ChangeNumReplicas")
 public class ChangeNumReplicasCommand extends AbstractBaseAdminCommand implements Command {
-  private static final Logger LOGGER = LoggerFactory.getLogger(StartBrokerCommand.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(ChangeNumReplicasCommand.class);
 
   @CommandLine.Option(names = {"-zkAddress"}, required = false, description = "HTTP address of Zookeeper.")
   private String _zkAddress = DEFAULT_ZK_ADDRESS;
diff --git a/pinot-tools/src/main/java/org/apache/pinot/tools/streams/AvroFileSourceGenerator.java b/pinot-tools/src/main/java/org/apache/pinot/tools/streams/AvroFileSourceGenerator.java
index bfe6e34a68..86831c4251 100644
--- a/pinot-tools/src/main/java/org/apache/pinot/tools/streams/AvroFileSourceGenerator.java
+++ b/pinot-tools/src/main/java/org/apache/pinot/tools/streams/AvroFileSourceGenerator.java
@@ -48,7 +48,7 @@ import org.slf4j.LoggerFactory;
  * time index based on row number.
  */
 public class AvroFileSourceGenerator implements PinotSourceDataGenerator {
-  private static final Logger LOGGER = LoggerFactory.getLogger(PinotRealtimeSource.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(AvroFileSourceGenerator.class);
   private DataFileStream<GenericRecord> _avroDataStream;
   private final Schema _pinotSchema;
   private long _rowsProduced;


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org