Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set MDC so that one can route minion task logs to separate files cleanly #9400

Merged
merged 3 commits into from
Sep 15, 2022
Merged

set MDC so that one can route minion task logs to separate files cleanly #9400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90a6136
set MDC so that one can route task logs to separate files
klsince Sep 14, 2022
86847fb
add an example log4j2.xml
klsince Sep 14, 2022
893870f
simplify as log4j2.xml is the real switch for log routing
klsince Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.apache.pinot.minion.executor.TaskExecutorFactoryRegistry;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;


/**
Expand Down Expand Up @@ -85,6 +86,8 @@ public TaskResult run() {
.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTimeMs - jobInQueueTimeMs,
TimeUnit.MILLISECONDS);
try {
// Set taskId in MDC so that one may config logger to route task logs to separate file.
MDC.put("taskId", _taskConfig.getId());
_minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, 1L);
return runInternal();
} finally {
Expand All @@ -93,6 +96,8 @@ public TaskResult run() {
_minionMetrics
.addPhaseTiming(taskType, MinionQueryPhase.TASK_EXECUTION, executionTimeMs, TimeUnit.MILLISECONDS);
LOGGER.info("Task: {} completed in: {}ms", _taskConfig.getId(), executionTimeMs);
// Clear taskId from MDC to reset it.
MDC.remove("taskId");
}
}

Expand Down
19 changes: 19 additions & 0 deletions pinot-tools/src/main/resources/log4j2.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,25 @@
</Policies>
<DefaultRolloverStrategy max="10"/>
</RollingFile>
<Routing name="taskLogs" ignoreExceptions="false">
<Routes pattern="${ctx:taskId}">
<Route>
<RollingFile
name="minionTaskLogs-${ctx:taskId}"
fileName="${env:LOG_ROOT}/minionTaskLogs-${ctx:taskId:-default}.log"
filePattern="${env:LOG_ROOT}/minionTaskLogs-%d{yyyy-MM-dd}-%i.log"
immediateFlush="true">
<PatternLayout pattern="${env:LOG_PATTERN}"/>
<Policies>
<SizeBasedTriggeringPolicy size="19500KB"/>
</Policies>
<DefaultRolloverStrategy max="50"/>
</RollingFile>
</Route>
</Routes>
<!-- Created appender TTL -->
<IdlePurgePolicy timeToLive="15" timeUnit="minutes"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i understood the rollover if > 19.5MB or rollover if > 50 files.
What's the 15 minutes TTL? will that be enough?
Also is 50 max good enough? We could have a lot more tasks running at once rt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per the doc: 15min TTL is used to close the dynamically created appender, after it's idle for up to TTL.

as to purge old log files, there are multiple options: by max count as in the example here or time based (which may be more suitable for prod use, e.g. to keep log files as long as the task info is kept in helix, which is 24hr by default)

</Routing>
</Appenders>
<Loggers>
<Root level="info" additivity="false">
Expand Down