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

crl-release-24.2: sstable: fix restarts integer overflow #4245

Open
wants to merge 1 commit into
base: crl-release-24.2
Choose a base branch
from

Conversation

EdwardX29
Copy link
Contributor

Fix bug with integer overflows while indexing into blocks with large KVs in SeekGE() and SeekLT() in rowblk_iter. Updated members in blockEntry that represent offsets in blocks to be type offsetInBlock (alias for int64).

Added check in rowblk_writer to ensure that block sizes do not exceed rowblk.MaximumSize before writing more data to the block.

Wrote unit tests to verify correct behavior for SeekGE() and SeekLT() with large blocks >2GB.

Fix bug with integer overflows while indexing into blocks with large KVs
in SeekGE() and SeekLT() in rowblk_iter. Updated members in blockEntry
that represent  offsets in blocks to be type offsetInBlock (alias for int64).

Added check in rowblk_writer to ensure that block sizes do not exceed
rowblk.MaximumSize before writing more data to the block.

Wrote unit tests to verify correct behavior for SeekGE() and SeekLT() with large
blocks >2GB.
@EdwardX29 EdwardX29 requested a review from a team as a code owner January 10, 2025 16:37
@EdwardX29 EdwardX29 requested a review from RaduBerinde January 10, 2025 16:37
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@EdwardX29 EdwardX29 requested a review from jbowens January 10, 2025 20:10
Copy link
Collaborator

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @RaduBerinde)

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