kfaraz commented on code in PR #15706:
URL: https://github.com/apache/druid/pull/15706#discussion_r1455509916
##########
sql/src/main/java/org/apache/druid/sql/calcite/parser/DruidSqlParserUtils.java:
##########
@@ -90,7 +90,9 @@ public static Granularity
convertSqlNodeToGranularityThrowingParseExceptions(Sql
throw e;
}
catch (Exception e) {
- log.debug(e, StringUtils.format("Unable to convert %s to a valid
granularity.", sqlNode.toString()));
+ if (log.isDebugEnabled()) {
+ log.debug(e, StringUtils.format("Unable to convert %s to a valid
granularity.", sqlNode.toString()));
+ }
Review Comment:
```suggestion
log.debug(e, "Unable to convert node[%s] to a valid granularity.",
sqlNode);
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java:
##########
@@ -752,7 +752,9 @@ TaskStatus runHashPartitionMultiPhaseParallel(TaskToolbox
toolbox) throws Except
);
// This is for potential debugging in case we suspect bad estimation
of cardinalities etc,
- LOG.debug("intervalToNumShards: %s", intervalToNumShards.toString());
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("intervalToNumShards: %s", intervalToNumShards.toString());
+ }
Review Comment:
Just removing the `.toString()` would be simpler here.
```suggestion
LOG.debug("intervalToNumShards: %s", intervalToNumShards);
```
##########
integration-tests/src/main/java/org/apache/druid/testing/clients/OverlordResourceTestClient.java:
##########
@@ -127,7 +127,9 @@ public TaskStatusPlus getTaskStatus(String taskID)
StringUtils.urlEncode(taskID)
)
);
- LOG.debug("Index status response" + response.getContent());
+ if (LOG.isDebugEnabled()) {
Review Comment:
I suppose the changes in this class are not really needed as it is just a
test class anyway. The changes wouldn't really have any impact.
##########
processing/src/main/java/org/apache/druid/java/util/common/guava/ParallelMergeCombiningSequence.java:
##########
@@ -475,20 +475,22 @@ private int computeNumTasks()
final int computedNumParallelTasks =
Math.max(computedOptimalParallelism, 1);
- LOG.debug(
- "Computed parallel tasks: [%s]; ForkJoinPool details - sequence
parallelism: [%s] "
- + "active threads: [%s] running threads: [%s] queued submissions:
[%s] queued tasks: [%s] "
- + "pool parallelism: [%s] pool size: [%s] steal count: [%s]",
- computedNumParallelTasks,
- parallelism,
- getPool().getActiveThreadCount(),
- runningThreadCount,
- submissionCount,
- getPool().getQueuedTaskCount(),
- getPool().getParallelism(),
- getPool().getPoolSize(),
- getPool().getStealCount()
- );
+ if (LOG.isDebugEnabled()) {
+ LOG.debug(
Review Comment:
Maybe also assign the result of `getPool` to a variable for simplicity and
reuse it in the log line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
