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

add metadata ttl field to DedupConfig #13636

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zhtaoxiang
Copy link
Contributor

Similar to UpsertConfig, we want to add a metadataTTL to DedupConfig. With this config, we can clean up stale dedup metadata.

This PR only adds this config. How to use this config to clean up the dedup metadata will be implemented in follow-up PRs

@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 35.19%. Comparing base (59551e4) to head (2690b8c).
Report is 773 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (59551e4) and HEAD (2690b8c). Click for more details.

HEAD has 10 uploads less than BASE
Flag BASE (59551e4) HEAD (2690b8c)
integration2 3 2
temurin 12 11
skip-bytebuffers-true 3 2
skip-bytebuffers-false 7 6
unittests 5 3
java-11 5 4
unittests2 3 0
Additional details and impacted files
@@              Coverage Diff              @@
##             master   #13636       +/-   ##
=============================================
- Coverage     61.75%   35.19%   -26.57%     
+ Complexity      207        6      -201     
=============================================
  Files          2436     2480       +44     
  Lines        133233   137038     +3805     
  Branches      20636    21305      +669     
=============================================
- Hits          82274    48226    -34048     
- Misses        44911    85163    +40252     
+ Partials       6048     3649     -2399     
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration <0.01% <0.00%> (-0.01%) ⬇️
integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration2 0.00% <0.00%> (ø)
java-11 35.15% <100.00%> (-26.55%) ⬇️
java-21 35.07% <100.00%> (-26.55%) ⬇️
skip-bytebuffers-false 35.17% <100.00%> (-26.58%) ⬇️
skip-bytebuffers-true 35.05% <100.00%> (+7.32%) ⬆️
temurin 35.19% <100.00%> (-26.57%) ⬇️
unittests 46.48% <100.00%> (-15.27%) ⬇️
unittests1 46.48% <100.00%> (-0.41%) ⬇️
unittests2 ?

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

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

@@ -52,4 +61,8 @@ public boolean isDedupEnabled() {
public String getMetadataManagerClass() {
return _metadataManagerClass;
}

public double getMetadataTTL() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this won't work for dedupManager since there is no support for removeExpiredPrimaryKey functionalities in the dedupPartitionManager.

Copy link
Contributor

Choose a reason for hiding this comment

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

do we plan to add this feature to dedupPartitionManager as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants