Copilot commented on code in PR #6797:
URL: https://github.com/apache/texera/pull/6797#discussion_r3629672740
##########
amber/src/main/python/core/runnables/main_loop.py:
##########
@@ -444,7 +444,7 @@ def _process_ecm(self, ecm_element: ECMElement):
ecm = ecm_element.payload
command = ecm.command_mapping.get(self.context.worker_id)
channel_id = self.context.current_input_channel_id
- logger.info(
+ logger.debug(
f"receive channel ECM from {channel_id}, id = {ecm.id}, cmd =
{command}"
)
Review Comment:
These DEBUG logs use f-strings, which eagerly format the message even when
DEBUG is disabled—this can still be costly on hot paths (per-message ECM
handling). Prefer lazy logging (e.g., logger.debug(\"... %s ...\", arg1, arg2,
...) depending on the logging framework) or guard formatting behind a
DEBUG-enabled check.
##########
amber/src/main/python/core/runnables/main_loop.py:
##########
@@ -453,7 +453,7 @@ def _process_ecm(self, ecm_element: ECMElement):
)
if self.context.ecm_manager.is_ecm_aligned(channel_id, ecm):
- logger.info(
+ logger.debug(
f"process channel ECM from {channel_id}, id = {ecm.id}, cmd =
{command}"
)
Review Comment:
These DEBUG logs use f-strings, which eagerly format the message even when
DEBUG is disabled—this can still be costly on hot paths (per-message ECM
handling). Prefer lazy logging (e.g., logger.debug(\"... %s ...\", arg1, arg2,
...) depending on the logging framework) or guard formatting behind a
DEBUG-enabled check.
##########
amber/src/main/scala/org/apache/texera/amber/engine/common/client/ClientActor.scala:
##########
@@ -156,6 +156,6 @@ private[client] class ClientActor extends Actor with
AmberLogging {
sender() ! Ack
coordinator ! x
case other =>
- logger.warn("client actor cannot handle " + other) //skip
+ logger.debug("client actor cannot handle " + other) //skip
Review Comment:
This DEBUG log concatenates strings eagerly (including other.toString) even
when DEBUG is disabled. Consider parameterized logging or guarding behind a
DEBUG-enabled check, especially if 'other' can be large or frequent.
##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/worker/DataProcessor.scala:
##########
@@ -268,7 +268,7 @@ class DataProcessor(
outputManager.flush(Some(downstreamChannelsInScope))
outputGateway.getActiveChannels.foreach { activeChannelId =>
if (downstreamChannelsInScope.contains(activeChannelId)) {
- logger.info(
+ logger.debug(
s"send ECM to $activeChannelId, id = ${ecm.id}, cmd = $command"
)
Review Comment:
These DEBUG statements use string interpolation, which eagerly builds
strings even when DEBUG is disabled. Since these are hot-path logs (ECM
receive/process/send), consider switching to parameterized logging (SLF4J-style
placeholders) or guarding with a DEBUG-enabled check to avoid unnecessary
allocations.
##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/worker/DataProcessor.scala:
##########
@@ -242,14 +242,14 @@ class DataProcessor(
): Unit = {
inputManager.currentChannelId = channelId
val command = ecm.commandMapping.get(actorId.name)
- logger.info(s"receive ECM from $channelId, id = ${ecm.id}, cmd = $command")
+ logger.debug(s"receive ECM from $channelId, id = ${ecm.id}, cmd =
$command")
if (ecm.ecmType != NO_ALIGNMENT) {
pauseManager.pauseInputChannel(ECMPause(ecm.id), List(channelId))
}
if (ecmManager.isECMAligned(channelId, ecm)) {
logManager.markAsReplayDestination(ecm.id)
// invoke the control command carried with the ECM
- logger.info(s"process ECM from $channelId, id = ${ecm.id}, cmd =
$command")
+ logger.debug(s"process ECM from $channelId, id = ${ecm.id}, cmd =
$command")
Review Comment:
These DEBUG statements use string interpolation, which eagerly builds
strings even when DEBUG is disabled. Since these are hot-path logs (ECM
receive/process/send), consider switching to parameterized logging (SLF4J-style
placeholders) or guarding with a DEBUG-enabled check to avoid unnecessary
allocations.
##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/worker/DataProcessor.scala:
##########
@@ -242,14 +242,14 @@ class DataProcessor(
): Unit = {
inputManager.currentChannelId = channelId
val command = ecm.commandMapping.get(actorId.name)
- logger.info(s"receive ECM from $channelId, id = ${ecm.id}, cmd = $command")
+ logger.debug(s"receive ECM from $channelId, id = ${ecm.id}, cmd =
$command")
Review Comment:
These DEBUG statements use string interpolation, which eagerly builds
strings even when DEBUG is disabled. Since these are hot-path logs (ECM
receive/process/send), consider switching to parameterized logging (SLF4J-style
placeholders) or guarding with a DEBUG-enabled check to avoid unnecessary
allocations.
##########
amber/src/main/python/core/runnables/main_loop.py:
##########
@@ -470,7 +470,7 @@ def _process_ecm(self, ecm_element: ECMElement):
active_channel_id
) in self.context.output_manager.get_output_channel_ids():
if active_channel_id in downstream_channels_in_scope:
- logger.info(
+ logger.debug(
f"send ECM to {active_channel_id},"
f" id = {ecm.id}, cmd = {command}"
)
Review Comment:
These DEBUG logs use f-strings, which eagerly format the message even when
DEBUG is disabled—this can still be costly on hot paths (per-message ECM
handling). Prefer lazy logging (e.g., logger.debug(\"... %s ...\", arg1, arg2,
...) depending on the logging framework) or guard formatting behind a
DEBUG-enabled check.
--
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]
