Skip to content

test: Write embedded test task logs to the main logger.#19428

Merged
gianm merged 3 commits into
apache:masterfrom
gianm:embedded-indexer-task-logs
May 9, 2026
Merged

test: Write embedded test task logs to the main logger.#19428
gianm merged 3 commits into
apache:masterfrom
gianm:embedded-indexer-task-logs

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented May 7, 2026

In production, Indexers write task logs to their own dedicated files in order to make them separately uploadable and viewable. In embedded tests, it's better to write task logs to the main log file, to keep all logging output for the test run in one place.

In production, Indexers write task logs to their own dedicated files
in order to make them separately uploadable and viewable. In embedded
tests, it's better to write task logs to the main log file, to keep
all logging output for the test run in one place.
Copy link
Copy Markdown
Member

@FrankChen021 FrankChen021 left a comment

Choose a reason for hiding this comment

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

Severity Findings
P0 0
P1 0
P2 1
P3 0
Total 1

This is an automated review by Codex GPT-5

final File taskFile = new File(taskDir, "task.json");
final File reportsFile = new File(attemptDir, "report.json");
final File logFile = new File(taskDir, "log");
final File logFile =
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[P2] Disabling task log files does not stop appenderator log routing

When druid.worker.useSeparateTaskLogFiles=false, the runner skips the task log file here, but ingestion tasks that create appenderators can still install per-task log routing through UnifiedIndexerAppenderatorsManager, whose override unconditionally calls Appenderators.setTaskThreadContextForIndexers(taskId, taskDirectory.getTaskLogFile(taskId)). BatchAppenderator and StreamAppenderator invoke that override on persist/push/background executor paths, so those logs can still be routed to per-task files instead of only the Indexer process log. The new flag needs to be honored by the appenderator manager/override as well.

@gianm gianm closed this May 8, 2026
@gianm gianm reopened this May 8, 2026
Copy link
Copy Markdown
Member

@FrankChen021 FrankChen021 left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 changed files for correctness, edge cases, concurrency, and integration risks; no issues found.


This is an automated review by Codex GPT-5

@gianm gianm merged commit 98ea27e into apache:master May 9, 2026
125 of 128 checks passed
@gianm gianm deleted the embedded-indexer-task-logs branch May 9, 2026 21:52
@github-actions github-actions Bot added this to the 38.0.0 milestone May 9, 2026
317brian pushed a commit to 317brian/druid that referenced this pull request May 11, 2026
In production, Indexers write task logs to their own dedicated files
in order to make them separately uploadable and viewable. In embedded
tests, it's better to write task logs to the main log file, to keep
all logging output for the test run in one place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants