Skip to content

Conversation

@tibrewalpratik17
Copy link
Contributor

label:

  • bugfix

Currently, this endpoint tables/{tableName}/validDocIdsMetadata returns the validDocID metadata for the same segment multiple times (equal to number of replicas).

This causes issues with UpsertCompaction task too as the same segment starts getting processed multiple times (for deletion / compaction).

This patch dedupes the response of the API itself which is implicitly used by UpsertCompactionTaskGenerator too.

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2024

Codecov Report

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

Comparison is base (1d490c1) 61.77% compared to head (177001c) 61.73%.

Files Patch % Lines
...t/controller/util/ServerSegmentMetadataReader.java 0.00% 8 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #12445      +/-   ##
============================================
- Coverage     61.77%   61.73%   -0.04%     
  Complexity      207      207              
============================================
  Files          2436     2436              
  Lines        133130   133134       +4     
  Branches      20623    20624       +1     
============================================
- Hits          82241    82191      -50     
- Misses        44850    44899      +49     
- Partials       6039     6044       +5     
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (ø)
integration <0.01% <0.00%> (ø)
integration1 <0.01% <0.00%> (ø)
integration2 0.00% <0.00%> (ø)
java-11 61.69% <0.00%> (+<0.01%) ⬆️
java-21 61.60% <0.00%> (-0.05%) ⬇️
skip-bytebuffers-false 61.72% <0.00%> (-0.01%) ⬇️
skip-bytebuffers-true 61.57% <0.00%> (-0.05%) ⬇️
temurin 61.73% <0.00%> (-0.04%) ⬇️
unittests 61.73% <0.00%> (-0.04%) ⬇️
unittests1 46.86% <ø> (-0.04%) ⬇️
unittests2 27.74% <0.00%> (-0.02%) ⬇️

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.

}
}

Map<String, ValidDocIdsMetadataInfo> dedupedValidDocIdsMapping = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of adding this, we can change validDocIdsMetadataInfos to Map<String, ValidDocIdsMetadataInfo>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure let me do that!

@snleee
Copy link
Contributor

snleee commented Feb 20, 2024

@tibrewalpratik17 can we combine the pr with #12431 ?

@tibrewalpratik17
Copy link
Contributor Author

@snleee closing this PR, will add all the changes in the other one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants