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

[ARCTIC-915][AMS] Orphan files clean service should not delete flink temporary metadata files #1181

Merged
merged 12 commits into from
Mar 7, 2023

Conversation

wangtaohz
Copy link
Contributor

@wangtaohz wangtaohz commented Mar 1, 2023

Why are the changes needed?

fix #915

Brief change log

  • not delete flink temporary metadata files which start with flink.job-id in the latest snapshot summary
  • refactor OrphanFilesCleanService, remove the useless execute/metadata/mode flag

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduces a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@github-actions github-actions bot added module:ams-server Ams server module module:ams-dashboard Ams dashboard module labels Mar 1, 2023
modify logs of OrphanFilesCleanService
use com.google.common.base.Strings#isNullOrEmpty
@codecov
Copy link

codecov bot commented Mar 2, 2023

Codecov Report

Patch coverage: 94.44% and project coverage change: -24.39 ⚠️

Comparison is base (325be4f) 52.78% compared to head (2db5710) 28.39%.

Additional details and impacted files
@@              Coverage Diff              @@
##             master    #1181       +/-   ##
=============================================
- Coverage     52.78%   28.39%   -24.39%     
- Complexity      524     5020     +4496     
=============================================
  Files            43      658      +615     
  Lines          3704    69216    +65512     
  Branches        354     7981     +7627     
=============================================
+ Hits           1955    19657    +17702     
- Misses         1620    47631    +46011     
- Partials        129     1928     +1799     
Flag Coverage Δ
core 27.02% <94.44%> (?)
trino 52.78% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...s/server/service/impl/OrphanFilesCleanService.java 83.95% <92.59%> (ø)
...c/optimizer/operator/executor/IcebergExecutor.java 92.40% <100.00%> (ø)
...tic/optimizer/operator/executor/MajorExecutor.java 91.07% <100.00%> (ø)
...tic/optimizer/operator/executor/MinorExecutor.java 92.85% <100.00%> (ø)
...park/sql/execution/CreateArcticTableLikeExec.scala 0.00% <0.00%> (ø)
...om/netease/arctic/scan/expressions/RewriteNot.java 66.66% <0.00%> (ø)
...tic/hive/io/writer/AdaptHiveOutputFileFactory.java 78.94% <0.00%> (ø)
...ease/arctic/ams/server/model/TableTaskHistory.java 0.00% <0.00%> (ø)
...y/authorization/AuthorizationPreEventListener.java 0.00% <0.00%> (ø)
...sar/common/schema/factories/AvroSchemaFactory.java 0.00% <0.00%> (ø)
... and 609 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

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

LGTM.

@zhoujinsong zhoujinsong merged commit 8626e75 into apache:master Mar 7, 2023
wangtaohz added a commit that referenced this pull request May 16, 2023
…uld not delete flink temporary metadata files (#1181)

* 1.refactor OrphanFilesCleanService, remove useless execute/metadata flag
2.not delete flink temporary metadata file

* fix checkstyle
refactor getExcludeFileNameRegex

* use io of UnkeyedTable instead of ArcticTable
modify logs of OrphanFilesCleanService
use com.google.common.base.Strings#isNullOrEmpty

* introduce a static variable for flink.job-id

* modify OrphanFilesCleanService test case

* add test case for coverage

* modify some logs for optimizer

---------

Co-authored-by: ZhouJinsong <zhoujinsong0505@163.com>
zhoujinsong added a commit that referenced this pull request May 31, 2023
…temporary metadata files (#1181)

* 1.refactor OrphanFilesCleanService, remove useless execute/metadata flag
2.not delete flink temporary metadata file

* fix checkstyle
refactor getExcludeFileNameRegex

* use io of UnkeyedTable instead of ArcticTable
modify logs of OrphanFilesCleanService
use com.google.common.base.Strings#isNullOrEmpty

* introduce a static variable for flink.job-id

* modify OrphanFilesCleanService test case

* add test case for coverage

* modify some logs for optimizer

---------

Co-authored-by: ZhouJinsong <zhoujinsong0505@163.com>
@wangtaohz wangtaohz deleted the fix-915 branch July 4, 2023 01:45
ShawHee pushed a commit to ShawHee/arctic that referenced this pull request Dec 29, 2023
…temporary metadata files (apache#1181)

* 1.refactor OrphanFilesCleanService, remove useless execute/metadata flag
2.not delete flink temporary metadata file

* fix checkstyle
refactor getExcludeFileNameRegex

* use io of UnkeyedTable instead of ArcticTable
modify logs of OrphanFilesCleanService
use com.google.common.base.Strings#isNullOrEmpty

* introduce a static variable for flink.job-id

* modify OrphanFilesCleanService test case

* add test case for coverage

* modify some logs for optimizer

---------

Co-authored-by: ZhouJinsong <zhoujinsong0505@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:ams-dashboard Ams dashboard module module:ams-server Ams server module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Flink can not restore from checkpoint if pending files are removed by Optimizer
2 participants