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.

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

Recreate new TaskInstance Working Directory when exist in worker #15358

Conversation

ruanwenjun
Copy link
Member

@ruanwenjun ruanwenjun commented Dec 24, 2023

Purpose of the pull request

Recreate the working directory, avoid the exist directory will affect the task instance execution.

TODO: We can put this logic in task plugin, but this will cause all task plugin need to handle this situation.

Brief change log

When we find the task instance working directory exist in worker, we will recreate it.

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

@codecov-commenter
Copy link

codecov-commenter commented Dec 24, 2023

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (9212531) 37.80% compared to head (7297e25) 37.82%.

❗ Current head 7297e25 differs from pull request most recent head c8b77bc. Consider uploading reports for the commit c8b77bc to get more accurate results

Files Patch % Lines
...server/worker/utils/TaskExecutionContextUtils.java 0.00% 15 Missing ⚠️
...duler/server/worker/runner/WorkerTaskExecutor.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev   #15358      +/-   ##
============================================
+ Coverage     37.80%   37.82%   +0.01%     
- Complexity     4680     4681       +1     
============================================
  Files          1304     1304              
  Lines         44934    44937       +3     
  Branches       4811     4812       +1     
============================================
+ Hits          16989    16997       +8     
+ Misses        26095    26088       -7     
- Partials       1850     1852       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ruanwenjun ruanwenjun added the improvement make more easy to user or prompt friendly label Dec 25, 2023
@EricGao888
Copy link
Member

TODO: We can put this logic in task plugin, but this will cause all task plugin need to handle this situation. What does this TODO mean? Doing it in WorkerTaskExecutor.java seems better than in task plugins.

@ruanwenjun
Copy link
Member Author

TODO: We can put this logic in task plugin, but this will cause all task plugin need to handle this situation. What does this TODO mean? Doing it in WorkerTaskExecutor.java seems better than in task plugins.

Some task instance may store some info in its working, and when we failover or recover(Flink) we may need to read this data.

But right now, we didn't have this kind of task plugin.

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_createWorkingDirectoryWhenExecuteTaskInstanceDev branch from 6082907 to c8b77bc Compare December 25, 2023 05:39
Copy link

sonarcloud bot commented Dec 25, 2023

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@ruanwenjun ruanwenjun merged commit 0e88ea3 into apache:dev Dec 25, 2023
52 checks passed
@ruanwenjun ruanwenjun deleted the dev_wenjun_createWorkingDirectoryWhenExecuteTaskInstanceDev branch December 25, 2023 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend improvement make more easy to user or prompt friendly ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants