Skip to content

[Improvement-13751][Worker] Support real-time pod log collection #13752

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

Jump to bottom
Merged
SbloodyS merged 6 commits into from
Apr 5, 2023
Merged

[Improvement-13751][Worker] Support real-time pod log collection #13752

SbloodyS merged 6 commits into from
Apr 5, 2023

Conversation

@Radeity
Copy link
Member

@Radeity Radeity commented Mar 15, 2023 •
edited
Loading

Purpose of the pull request

Brief change log

  • Use LogWatch to collect real-time pod logs.
  • Add EMPTY_STRING ("") after clear logBuffer to make log format clearer.
[INFO] 2023-03-16 10:28:24.923 +0800 -  -> 
	23/03/16 10:28:24 INFO LoggingPodStatusWatcherImpl: Application status for spark-fdb9e53137854ea2ab6fc6c9a15761ac (phase: Running)
[INFO] 2023-03-16 10:28:25.924 +0800 -  -> 
	23/03/16 10:28:25 INFO LoggingPodStatusWatcherImpl: Application status for spark-fdb9e53137854ea2ab6fc6c9a15761ac (phase: Running)
[INFO] 2023-03-16 10:28:26.924 +0800 -  -> 
	[K8S-pod-log-spark-pi]: 23/03/16 02:28:25 INFO KubernetesClusterSchedulerBackend$KubernetesDriverEndpoint: Registered executor NettyRpcEndpointRef(spark-client://Executor) (10.244.1.211:59826) with ID 2,  ResourceProfileId 0
	[K8S-pod-log-spark-pi]: 23/03/16 02:28:25 INFO BlockManagerMasterEndpoint: Registering block manager 10.244.1.211:36305 with 1048.8 MiB RAM, BlockManagerId(2, 10.244.1.211, 36305, None)
	[K8S-pod-log-spark-pi]: 23/03/16 02:28:26 INFO KubernetesClusterSchedulerBackend$KubernetesDriverEndpoint: Registered executor NettyRpcEndpointRef(spark-client://Executor) (10.244.2.151:36974) with ID 1,  ResourceProfileId 0
	[K8S-pod-log-spark-pi]: 23/03/16 02:28:26 INFO KubernetesClusterSchedulerBackend: SchedulerBackend is ready for scheduling beginning after reached minRegisteredResourcesRatio: 0.8
	[K8S-pod-log-spark-pi]: 23/03/16 02:28:26 INFO BlockManagerMasterEndpoint: Registering block manager 10.244.2.151:42145 with 1048.8 MiB RAM, BlockManagerId(1, 10.244.2.151, 42145, None)
	[K8S-pod-log-spark-pi]: 23/03/16 02:28:26 INFO SparkContext: Starting job: reduce at SparkPi.scala:38
	[K8S-pod-log-spark-pi]: 23/03/16 02:28:26 INFO DAGScheduler: Got job 0 (reduce at SparkPi.scala:38) with 10 output partitions

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@Radeity Radeity requested a review from kezhenxu94 as a code owner March 15, 2023 13:58
@Radeity Radeity closed this Mar 15, 2023
@Radeity Radeity reopened this Mar 15, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 16, 2023 •
edited
Loading

Codecov Report

Merging #13752 (d8b246e) into dev (aa79c01) will increase coverage by 0.01%.
The diff coverage is 25.14%.

❗ Current head d8b246e differs from pull request most recent head 08d926f. Consider uploading reports for the commit 08d926f to get more accurate results

@@             Coverage Diff              @@
##                dev   #13752      +/-   ##
============================================
+ Coverage     39.06%   39.07%   +0.01%     
- Complexity     4423     4435      +12     
============================================
  Files          1131     1142      +11     
  Lines         42121    42033      -88     
  Branches       4776     4742      -34     
============================================
- Hits          16455    16425      -30     
+ Misses        23849    23807      -42     
+ Partials       1817     1801      -16
Impacted Files Coverage Δ
...olphinscheduler/plugin/alert/email/MailSender.java 48.97% <0.00%> (-1.71%) ⬇️
.../dolphinscheduler/alert/AlertRequestProcessor.java 0.00% <0.00%> (-93.75%) ⬇️
...org/apache/dolphinscheduler/alert/AlertServer.java 0.00% <0.00%> (-45.95%) ⬇️
.../dolphinscheduler/api/aspect/CacheEvictAspect.java 5.12% <0.00%> (ø)
...low/instance/pause/pause/PauseExecuteFunction.java 0.00% <0.00%> (ø)
...or/workflow/instance/stop/StopExecuteFunction.java 0.00% <0.00%> (ø)
.../apache/dolphinscheduler/api/rpc/ApiRpcClient.java 60.00% <0.00%> (ø)
...eduler/api/service/impl/DataSourceServiceImpl.java 50.45% <0.00%> (ø)
...er/api/service/impl/MetricsCleanUpServiceImpl.java 14.28% <0.00%> (ø)
...r/api/service/impl/ProcessInstanceServiceImpl.java 59.92% <0.00%> (ø)
... and 145 more

... and 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Radeity Radeity force-pushed the Improvement-13751 branch from c922d71 to 536c972 Compare March 17, 2023 15:59
@Radeity Radeity mentioned this pull request Mar 19, 2023
Closed
3 tasks
@SbloodyS SbloodyS added feature new feature 3.2.0 for 3.2.0 version labels Mar 22, 2023
@SbloodyS SbloodyS added this to the 3.2.0 milestone Mar 22, 2023
SbloodyS
SbloodyS reviewed Mar 22, 2023
View reviewed changes
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

I've rerun the CI.

@Radeity Radeity closed this Mar 22, 2023
@Radeity Radeity reopened this Mar 22, 2023
@Radeity
Copy link
Member Author

Radeity commented Mar 22, 2023

I've rerun the CI.

Thx, but ... it runs failed, please help rerun it again.

SbloodyS
SbloodyS previously approved these changes Mar 23, 2023
View reviewed changes
@Radeity
Copy link
Member Author

Radeity commented Mar 25, 2023

Hi, @ruanwenjun , would you like to double check?

caishunfeng
caishunfeng reviewed Mar 28, 2023
View reviewed changes
...k-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractCommandExecutor.java Outdated
String msg = (String) podLogOutputFuture.get();
if (StringUtils.isEmpty(msg)) {
// delete pod after successful execution
ProcessUtils.cancelApplication(taskRequest);
Copy link
Contributor

@caishunfeng caishunfeng Mar 28, 2023

Choose a reason for hiding this comment

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

Why need to cancel when pod log output is empty? Does it necessarily mean that the pod is finished?

Copy link
Member Author

@Radeity Radeity Mar 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The pod is deleted in cancelApplication only when task runs successfully and we collect whole logs (empty msg).

@Radeity @caishunfeng
Update dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/mai…
0562227 
…n/java/org/apache/dolphinscheduler/plugin/task/api/AbstractCommandExecutor.java

Co-authored-by: caishunfeng <[email protected]>
@Radeity Radeity closed this Mar 30, 2023
@Radeity Radeity reopened this Mar 30, 2023
@Radeity Radeity marked this pull request as draft March 30, 2023 07:32
@Radeity Radeity marked this pull request as ready for review March 30, 2023 09:08
@Radeity
Copy link
Member Author

Radeity commented Mar 30, 2023

Hi, @caishunfeng , i've made some changes to make the exception processing logic cleaner. Would you like to help review it again when you're available :D

@Radeity Radeity changed the title [Improvement-13751] Support real-time pod log collection [Improvement-13751][Worker] Support real-time pod log collection Apr 1, 2023
caishunfeng
caishunfeng approved these changes Apr 4, 2023
View reviewed changes
Copy link
Contributor

@caishunfeng caishunfeng left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 4, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 11 Code Smells

34.6% 34.6% Coverage
0.0% 0.0% Duplication

@SbloodyS SbloodyS merged commit d6fcf60 into apache:dev Apr 5, 2023
@Radeity Radeity mentioned this pull request Jun 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@caishunfeng caishunfeng caishunfeng approved these changes

@SbloodyS SbloodyS SbloodyS left review comments

@zhuangchong zhuangchong Awaiting requested review from zhuangchong

@ruanwenjun ruanwenjun Awaiting requested review from ruanwenjun ruanwenjun is a code owner

@kezhenxu94 kezhenxu94 Awaiting requested review from kezhenxu94

Assignees

@Radeity Radeity

Labels

3.2.0 for 3.2.0 version backend feature new feature

Projects

None yet

Milestone

3.2.0

Development

Successfully merging this pull request may close these issues.

[Improvement][Worker] Support real-time pod log collection

4 participants

@Radeity @codecov-commenter @caishunfeng @SbloodyS