Skip to content

MAHOUT-743: Add regression test for async amplitude chunk bounds - #773

Closed
Rutuja123-dos wants to merge 0 commit into
apache:dev-qdpfrom
Rutuja123-dos:fix-743-async-amplitude-oob
Closed

MAHOUT-743: Add regression test for async amplitude chunk bounds#773
Rutuja123-dos wants to merge 0 commit into
apache:dev-qdpfrom
Rutuja123-dos:fix-743-async-amplitude-oob

Conversation

@Rutuja123-dos

@Rutuja123-dos Rutuja123-dos commented Dec 30, 2025

Copy link
Copy Markdown
Contributor

Purpose of PR

Adds a regression test to ensure async amplitude encoding respects 'chunk_len'
instead of writing the full 'state_len', preventing out-of-bounds writes for
non-first chunks.

Related Issues or PRs

Changes Made

  • Bug fix
  • New feature
  • Refactoring
  • Documentation
  • Test
  • CI/CD pipeline
  • Other

Breaking Changes

  • Yes
  • No

Checklist

  • Added or updated unit tests for all changes
  • Added or updated documentation for all changes
  • Successfully built and ran all unit tests or manual tests locally
  • PR title follows "MAHOUT-XXX: Brief Description" format (if related to an issue)
  • Code follows ASF guidelines

@rich7420

Copy link
Copy Markdown
Contributor

@Rutuja123-dos thanks for the patch , you should aim to dev-qdp branch.

@Rutuja123-dos

Copy link
Copy Markdown
Contributor Author

@rich7420 thanks for clarification. I will retarget the PR to the dev-qdp branch

@Rutuja123-dos
Rutuja123-dos force-pushed the fix-743-async-amplitude-oob branch from 4cd7890 to c43905d Compare December 31, 2025 15:29
@Rutuja123-dos
Rutuja123-dos changed the base branch from main to dev-qdp December 31, 2025 15:36

@guan404ming guan404ming left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like this PR includes several changes that are unrelated to its main purpose. Could you please remove those so the scope stays focused? Thanks!

@ryankert01

Copy link
Copy Markdown
Member

@Rutuja123-dos How's it going so far? Any questions?

@Rutuja123-dos

Copy link
Copy Markdown
Contributor Author

Hi @ryankert01, apologies for the delay! I was busy with my university semester exams over the last two weeks, so I couldn't get to this sooner.

I am back now and will be working on cleaning up the PR to remove the unrelated commits and focus strictly on MAHOUT-743. I’ll have the updated, clean version pushed by tomorrow. Thanks for your patience

@Rutuja123-dos
Rutuja123-dos force-pushed the fix-743-async-amplitude-oob branch 3 times, most recently from 4acd154 to 0a4c008 Compare January 16, 2026 09:25
@Rutuja123-dos

Copy link
Copy Markdown
Contributor Author

@ryankert01 I have now updated the PR with a clean force-push. It now contains only the regression test for MAHOUT-743 with proper formatting and license headers. Ready for review. let me know if there should be any changes. Thanks!

@ryankert01 ryankert01 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please fix the ci errors and determine where your test should locate carefully.

@Rutuja123-dos
Rutuja123-dos force-pushed the fix-743-async-amplitude-oob branch 2 times, most recently from 3a93eb5 to 65b6863 Compare January 16, 2026 14:29
@Rutuja123-dos
Rutuja123-dos force-pushed the fix-743-async-amplitude-oob branch from 65b6863 to dd3856b Compare January 16, 2026 15:11
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.

[QDP] Fix async amplitude pipeline out-of-bounds write

4 participants