You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/09/14 22:13:29 UTC
[GitHub] [pinot] klsince opened a new pull request, #9400: set MDC so that one can route minion task logs to separate files cleanly
klsince opened a new pull request, #9400:
URL: https://github.com/apache/pinot/pull/9400
Allow one to route `all` logs (including exception stack trace) emit during a minion task execution to a separate file, so that the task logs can be accessed via the download/log restful API added recently. This method can capture the task logs completely (assuming the task is done by a single thread on minion worker which is the case today).
Added an example log4j2.xml for reference.
Alternatively, we might search logs from the default log file with the taskId, but that wouldn't work cleanly. One, not all task logs are tagged with taskId; two, the exception stacktrace would be missed.
## Release note
1. added a config pinot.minion.separate.task.logs - to turn this feature on/off.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] npawar commented on a diff in pull request #9400: set MDC so that one can route minion task logs to separate files cleanly
Posted by GitBox <gi...@apache.org>.
npawar commented on code in PR #9400:
URL: https://github.com/apache/pinot/pull/9400#discussion_r971425167
##########
pinot-tools/src/main/resources/log4j2.xml:
##########
@@ -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"/>
Review Comment:
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?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] klsince commented on pull request #9400: set MDC so that one can route minion task logs to separate files cleanly
Posted by GitBox <gi...@apache.org>.
klsince commented on PR #9400:
URL: https://github.com/apache/pinot/pull/9400#issuecomment-1247354842
example:
```
➜ pinot-oss git:(allow_separate_task_logs) ✗ ls -l ../logs
total 16736
-rw-r--r-- 1 xiaobing staff 5966 Sep 14 14:51 minionTaskLogs-Task_RealtimeToOfflineSegmentsTask_10102576-6dd0-416b-80e0-d25cc9ab65f8_1663192300019_0.log
-rw-r--r-- 1 xiaobing staff 5966 Sep 14 14:52 minionTaskLogs-Task_RealtimeToOfflineSegmentsTask_198d2193-89ce-4c4c-a576-8fa5b04d702d_1663192330009_0.log
...
-rw-r--r-- 1 xiaobing staff 6845 Sep 14 14:53 minionTaskLogs-default.log
-rw-r--r-- 1 xiaobing staff 7757824 Sep 14 15:13 pinot-all.log
```
```
➜ pinot-oss git:(allow_separate_task_logs) ✗ curl -X GET "http://localhost:9000/loggers/instances/Minion_192.168.0.106_6000/download?filePath=minionTaskLogs-Task_RealtimeToOfflineSegmentsTask_10102576-6dd0-416b-80e0-d25cc9ab65f8_1663192300019_0.log" -H "accept: application/octet-stream"
...
2022/09/14 14:51:40.308 INFO [TaskFactoryRegistry] [TaskStateModelFactory-task_thread-0] Start running
...
2022/09/14 14:51:40.419 ERROR [TaskFactoryRegistry] [TaskStateModelFactory-task_thread-0] Caught exception while executing task: Task_RealtimeToOfflineSegmentsTask_10102576-6dd0-416b-80e0-d25cc9ab65f8_1663192300019_0
java.lang.IllegalStateException: Failed to find table config for table: githubEvents_OFFLINE
at com.google.common.base.Preconditions.checkState(Preconditions.java:518) ~[guava-20.0.jar:?]
...
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
at java.lang.Thread.run(Thread.java:829) [?:?]
2022/09/14 14:51:40.419 INFO [TaskFactoryRegistry] [TaskStateModelFactory-task_thread-0] Task: Task_RealtimeToOfflineSegmentsTask_10102576-6dd0-416b-80e0-d25cc9ab65f8_1663192300019_0 completed in: 111ms
```
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #9400: set MDC so that one can route minion task logs to separate files cleanly
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged PR #9400:
URL: https://github.com/apache/pinot/pull/9400
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] klsince commented on a diff in pull request #9400: set MDC so that one can route minion task logs to separate files cleanly
Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #9400:
URL: https://github.com/apache/pinot/pull/9400#discussion_r971465896
##########
pinot-tools/src/main/resources/log4j2.xml:
##########
@@ -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"/>
Review Comment:
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)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #9400: set MDC so that one can route minion task logs to separate files cleanly
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9400:
URL: https://github.com/apache/pinot/pull/9400#issuecomment-1247381530
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9400?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#9400](https://codecov.io/gh/apache/pinot/pull/9400?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (86847fb) into [master](https://codecov.io/gh/apache/pinot/commit/b777a620c48c542fb6e42e04da4eed1b9cb5992d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b777a62) will **decrease** coverage by `2.75%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #9400 +/- ##
============================================
- Coverage 69.67% 66.91% -2.76%
- Complexity 4787 4895 +108
============================================
Files 1885 1402 -483
Lines 100419 72932 -27487
Branches 15283 11687 -3596
============================================
- Hits 69966 48804 -21162
+ Misses 25490 20576 -4914
+ Partials 4963 3552 -1411
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `66.91% <ø> (-0.02%)` | :arrow_down: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9400?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `27.69% <ø> (ø)` | |
| [...va/org/apache/pinot/core/routing/RoutingTable.java](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yb3V0aW5nL1JvdXRpbmdUYWJsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ache/pinot/server/access/AccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL2FjY2Vzcy9BY2Nlc3NDb250cm9sRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/TableDeletionMessage.java](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvVGFibGVEZWxldGlvbk1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [723 more](https://codecov.io/gh/apache/pinot/pull/9400/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] klsince commented on a diff in pull request #9400: set MDC so that one can route minion task logs to separate files cleanly
Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #9400:
URL: https://github.com/apache/pinot/pull/9400#discussion_r971375414
##########
pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java:
##########
@@ -85,6 +89,11 @@ public TaskResult run() {
.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTimeMs - jobInQueueTimeMs,
TimeUnit.MILLISECONDS);
try {
+ if (separateTaskLogs) {
+ LOGGER.info("Routing logs to separate file for task: {}", _taskConfig.getId());
+ // Setting MDC so that logger can route task logs to a separate file.
+ MDC.put("taskId", _taskConfig.getId());
Review Comment:
good question, this will run on its own thread, as you see it's in a closure to be called later
```
TaskFactory taskFactory = context -> {
```
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] npawar commented on a diff in pull request #9400: set MDC so that one can route minion task logs to separate files cleanly
Posted by GitBox <gi...@apache.org>.
npawar commented on code in PR #9400:
URL: https://github.com/apache/pinot/pull/9400#discussion_r971371736
##########
pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java:
##########
@@ -85,6 +89,11 @@ public TaskResult run() {
.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTimeMs - jobInQueueTimeMs,
TimeUnit.MILLISECONDS);
try {
+ if (separateTaskLogs) {
+ LOGGER.info("Routing logs to separate file for task: {}", _taskConfig.getId());
+ // Setting MDC so that logger can route task logs to a separate file.
+ MDC.put("taskId", _taskConfig.getId());
Review Comment:
if all tasks running put/remove the same key "taskId" how does it know to differentiate and not overwrite?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org