Skip to content

Fix for overflowing localInterval in SegmentCostCache.Bucket#12257

Merged
kfaraz merged 7 commits intoapache:masterfrom
tejaswini-imply:test-allSegmentGranularity-cachingCostBalancer
Feb 17, 2022
Merged

Fix for overflowing localInterval in SegmentCostCache.Bucket#12257
kfaraz merged 7 commits intoapache:masterfrom
tejaswini-imply:test-allSegmentGranularity-cachingCostBalancer

Conversation

@tejaswini-imply
Copy link
Member

@tejaswini-imply tejaswini-imply commented Feb 14, 2022

Combination of cachingCost balancer + all segmentGranularity + no interval not loading segments after ingestion

  • Combination is creating Segments of ETERNITY Interval (start - Long.MIN_VALUE/2, end - Long.MAX_VALUE/2)
  • Joint cost calculation of these segments in SegmentCostCache buckets is posing the problem as the buckets are designed in such a way to have bucket_interval_start < segment_interval_start.
  • For cost calculation start and end interval of segments are converted considering bucket_interval_start = 0 by taking difference between segment_interval_end, bucket_interval_start and normalised with MILLIS_FACTOR (millis_in_day/ log2), as a result, segment end-time conversion difference is overflowing(Long value).
  • The current PR accounts for this overflowing case as it normalises segment interval, bucket_interval_start with MILLIS_FACTOR first (since MILLIS_FACTOR is of order 10^8) and then takes the difference.

Key changed/added classes in this PR
  • SegmentsCostCache.Bucket
  • LoadRuleTest

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@tejaswini-imply tejaswini-imply changed the title Test loading no interval segments with cachingCostBalancer Fix for combination of cachingCost balancer + all segmentGranularity not loading segments after ingestion Feb 15, 2022
@tejaswini-imply tejaswini-imply marked this pull request as ready for review February 15, 2022 11:06
@tejaswini-imply tejaswini-imply changed the title Fix for combination of cachingCost balancer + all segmentGranularity not loading segments after ingestion Fix for overflowing localInterval in SegmentCostCache.Bucket Feb 15, 2022
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

LGTM.

@kfaraz
Copy link
Contributor

kfaraz commented Feb 17, 2022

Merging as CI failure is unrelated.

@kfaraz kfaraz merged commit 70c40c4 into apache:master Feb 17, 2022
@abhishekagarwal87 abhishekagarwal87 added this to the 0.23.0 milestone May 11, 2022
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.

4 participants