-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix bug to return validDocIDsMetadata from all servers #12431
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #12431 +/- ##
============================================
- Coverage 61.77% 61.75% -0.02%
Complexity 207 207
============================================
Files 2436 2436
Lines 133130 133176 +46
Branches 20623 20632 +9
============================================
+ Hits 82241 82246 +5
- Misses 44850 44890 +40
- Partials 6039 6040 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. @tibrewalpratik17 : can you also add what was the impact of this bug in the description?
@snleee can you take a look at this fix for the tagged issue? |
@tibrewalpratik17 can we combine this pr with #12445? |
4dbfd51
to
c987ff1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for catching the issue.
label:
bugfix
Fixes #12423
We were sending
serverToEndpoints
mapping instead ofendpointsToServer
which is expected by CompletionService. That's why CompletionService was always returning 1 server's response as it was having null as the only key in map and returning response of the last server it receives overwriting the response of previous server(s) always.The impact of this bug was that getValidDocIdMetadata endpoint was always returning results from one server instead of the number of servers the table is actually hosted on. This affects UpsertCompaction task too as it now processes the segments of any 1 random server at a time instead of all.
Another bug fix from #12445
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.