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

fix: issue of last partition not included for certain groups weightage #104

Merged
merged 3 commits into from
Dec 19, 2024

Conversation

kotharironak
Copy link
Contributor

Description

This PR addresses an issue caused by floating-point rounding to integers, which was leading to the last partition being excluded in the partitioners.

For example, with the following configuration, the default partitioner groups the weight range as:

  • Weight Range: 0.8039215686274509 to 0.99999999999

Due to the rounding issue at 0.99999999999, only partitions 41 to 62 are included for this group, out of 64 total topic partitions.

This PR fixes the rounding issue, ensuring all partitions are correctly included in the partitioner.

{ 
      "name": "spans",
      "groups": [
        {
          "name": "dynamic-group-0",
          "memberIds": [
            "test-1"
          ],
          "weight": 37
        },
        {
          "name": "dynamic-group-1",
          "memberIds": [
            "test-2"
          ],
          "weight": 29
        },
        {
          "name": "dynamic-group-2",
          "memberIds": [
            "test-2"
          ],
          "weight": 16
        }
      ],
      "defaultGroupWeight": 20
    }

Testing

see the test : testWithNonMultipleRoundingWeightRatio

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

Documentation

Make sure that you have documented corresponding changes in this repository or hypertrace docs repo if required.

@kotharironak kotharironak requested a review from a team as a code owner December 4, 2024 11:00
@kotharironak kotharironak changed the title fix: issue of last partition not included for certain groups weightags fix: issue of last partition not included for certain groups weightage Dec 4, 2024
Copy link

github-actions bot commented Dec 4, 2024

Test Results

15 files  ±0  15 suites  ±0   30s ⏱️ ±0s
68 tests +1  68 ✅ +1  0 💤 ±0  0 ❌ ±0 
86 runs  +1  86 ✅ +1  0 💤 ±0  0 ❌ ±0 

Results for commit 46e866a. ± Comparison against base commit db1bc29.

This pull request removes 7 and adds 8 tests. Note that renamed tests count towards both.
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [1] {application.id=app-2, rocksdb.compaction.universal.max.size.amplification.percent=50, rocksdb.compaction.universal.compression.size.percent=40, rocksdb.compaction.style=UNIVERSAL, rocksdb.periodic.compaction.seconds=60}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [1] {rocksdb.log.level=INFO_LEVEL, application.id=app-1, rocksdb.compaction.style=LEVEL, rocksdb.max.write.buffers=2, rocksdb.direct.reads.enabled=true, rocksdb.write.buffer.size=8388608, rocksdb.block.size=8388608, rocksdb.compression.type=SNAPPY_COMPRESSION}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [2] {rocksdb.log.level=DEBUG_LEVEL, application.id=app-2, rocksdb.compaction.style=UNIVERSAL, rocksdb.max.write.buffers=3, rocksdb.direct.reads.enabled=true, rocksdb.write.buffer.size=8388607, rocksdb.block.size=8388609, rocksdb.compression.type=SNAPPY_COMPRESSION}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [3] {rocksdb.cache.high.priority.pool.ratio=-0.1, application.id=app-3}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [3] {rocksdb.log.level=ERROR_LEVEL, application.id=app-3, rocksdb.compaction.style=FIFO, rocksdb.max.write.buffers=4, rocksdb.direct.reads.enabled=false, rocksdb.write.buffer.size=8388609, rocksdb.block.size=8388607, rocksdb.compression.type=SNAPPY_COMPRESSION}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [4] {rocksdb.cache.high.priority.pool.ratio=1.1, application.id=app-2}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [5] {rocksdb.cache.write.buffers.ratio=0.9, rocksdb.cache.high.priority.pool.ratio=0.2, application.id=app-5}
org.hypertrace.core.kafkastreams.framework.partitioner.WeightedGroupPartitionerTest ‑ testWithNonMultipleRoundingWeightRatio()
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [1] {rocksdb.block.size=8388608, rocksdb.write.buffer.size=8388608, rocksdb.direct.reads.enabled=true, rocksdb.max.write.buffers=2, rocksdb.compaction.style=LEVEL, application.id=app-1, rocksdb.log.level=INFO_LEVEL, rocksdb.compression.type=SNAPPY_COMPRESSION}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [1] {rocksdb.compaction.universal.compression.size.percent=40, rocksdb.compaction.universal.max.size.amplification.percent=50, application.id=app-2, rocksdb.periodic.compaction.seconds=60, rocksdb.compaction.style=UNIVERSAL}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [2] {rocksdb.block.size=8388609, rocksdb.write.buffer.size=8388607, rocksdb.direct.reads.enabled=true, rocksdb.max.write.buffers=3, rocksdb.compaction.style=UNIVERSAL, application.id=app-2, rocksdb.log.level=DEBUG_LEVEL, rocksdb.compression.type=SNAPPY_COMPRESSION}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [3] {application.id=app-3, rocksdb.cache.high.priority.pool.ratio=-0.1}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [3] {rocksdb.block.size=8388607, rocksdb.write.buffer.size=8388609, rocksdb.direct.reads.enabled=false, rocksdb.max.write.buffers=4, rocksdb.compaction.style=FIFO, application.id=app-3, rocksdb.log.level=ERROR_LEVEL, rocksdb.compression.type=SNAPPY_COMPRESSION}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [4] {application.id=app-2, rocksdb.cache.high.priority.pool.ratio=1.1}
org.hypertrace.core.kafkastreams.framework.rocksdb.BoundedMemoryConfigSetterTest ‑ [5] {rocksdb.cache.high.priority.pool.ratio=0.2, rocksdb.cache.write.buffers.ratio=0.9, application.id=app-5}

♻️ This comment has been updated with latest results.

…/java/org/hypertrace/core/kafkastreams/framework/partitioner/WeightedGroupPartitionerTest.java

Co-authored-by: Ravi Singal <[email protected]>
@laxmanchekka laxmanchekka merged commit 4a1bec0 into main Dec 19, 2024
5 checks passed
@laxmanchekka laxmanchekka deleted the test-group-partitioner branch December 19, 2024 05:30
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.

3 participants