Yicong-Huang commented on code in PR #6797:
URL: https://github.com/apache/texera/pull/6797#discussion_r3648118486
##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/scheduling/RegionExecutionManager.scala:
##########
@@ -210,7 +210,7 @@ class RegionExecutionManager(
// 3. Log whether the kills were successful
gracefulStopRequests.transform {
case Return(_) =>
- logger.info(s"Region ${region.id.id} successfully terminated.")
+ logger.debug(s"Region ${region.id.id} successfully terminated.")
Review Comment:
I want to keep this info level. not too many regions for each workflow. so
the verbosity is fine
##########
.github/workflows/build.yml:
##########
@@ -291,6 +291,14 @@ jobs:
AMBER_TEST_FILTER: skip-integration
# unit job uses its provisioned postgres catalog; default (rest)
needs a Lakekeeper not run here
STORAGE_ICEBERG_CATALOG_TYPE: postgres
+ # Backstop for CI log volume: the chatty per-worker/per-message
engine
+ # logs are DEBUG in source; pin the JVM root and the spawned Python
UDF
+ # workers so nothing at INFO leaks into the CI console. Mind the
spelling:
+ # logback's level is WARN, but loguru (the Python worker) only knows
+ # WARNING and raises ValueError on "WARN" — which crashes the worker
at
+ # startup before it hands its port back, hanging the whole job.
+ TEXERA_SERVICE_LOG_LEVEL: WARN
+ UDF_PYTHON_LOG_STREAMHANDLER_LEVEL: WARNING
Review Comment:
I would actually prefer to have more logs in CI, so when something fails we
can pinpoint it without running it again. Is this change necessary?
##########
.github/workflows/build.yml:
##########
@@ -330,6 +338,10 @@ jobs:
# cutting lints (scalafmt / scalafix) and the amber dist + binary
# license check stay in `amber`; this job is tests-only.
if: ${{ inputs.run_amber_integration }}
+ # A Python UDF worker that fails to start (e.g. a bad log level) leaves the
+ # JVM blocked on the proxy handshake with no internal timeout, so the whole
+ # job would otherwise hang until GitHub's 6h cap. Fail fast instead.
+ timeout-minutes: 40
Review Comment:
20 should be enough？
##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/common/PekkoActorRefMappingService.scala:
##########
@@ -76,13 +76,13 @@ class PekkoActorRefMappingService(actorService:
PekkoActorService) extends Amber
def removeActorRef(id: ActorVirtualIdentity): Unit = {
if (actorRefMapping.contains(id)) {
val ref = actorRefMapping.remove(id).get
- logger.warn(s"actor $id is not reachable anymore, it might have crashed.
old ref = $ref")
+ logger.debug(s"removed actor ref for $id. old ref = $ref")
Review Comment:
this changes the log's meaning. can we keep the log message?
--
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]
