Skip to content

[To rel/1.1]Fix WrappedSegment extension with big name on edge case#9484

Merged
MarcosZyk merged 2 commits intoapache:rel/1.1from
bigreybear:fix_bmec_1.1
Mar 31, 2023
Merged

[To rel/1.1]Fix WrappedSegment extension with big name on edge case#9484
MarcosZyk merged 2 commits intoapache:rel/1.1from
bigreybear:fix_bmec_1.1

Conversation

@bigreybear
Copy link
Contributor

Description

In determining the size of the pre-allocated segment for an EntityNode, an accurate calculation is performed by examining the children of the EntityNode if it has fewer than 20 children. Alternatively, the size is estimated using a hard-coded step list based on the cardinality of the EntityNode's children.

During the flushing process of the EntityNode, the segment size is assessed prior to the insertion of each child, and the segment is re-allocated or extended according to the anticipated size. If the segment is unable to accommodate the inserted child even after an extension, an exception will be raised, indicating that the record is too large or the extension has failed.

This issue arises in an edge case where the EntityNode has fewer than 20 children, each with a large size (approximately 1000 bytes in total) when calculated for pre-allocation. However, before the EntityNode is flushed, a new child with a large name and a small buffer, which includes aliases and other schema data but measures under 20 bytes, for instance, is inserted. The segment size assessment may be misled, resulting in a minor increment for the segment extension, e.g., from 1000 bytes to 1022 bytes, leading to an overflow since the segment cannot accommodate the large name.

This hotfix takes into account the length of the name, bytes of the name, and the record buffer offset when assessing the expected size of the WrappedSegment. This issue will not occur with AliasIndexPage or InternalPage, as their lengths will never be assessed multiple times due to their fixed length. The root cause of the original issue, as well as potential performance improvements, can be attributed to the design of the WrappedSegment increment, which is determined by a hard-coded step list as aforementioned.

@bigreybear bigreybear changed the title Fix WrappedSegment extension with big name on edge case [To rel/1.1]Fix WrappedSegment extension with big name on edge case Mar 30, 2023
Copy link
Member

@wangchao316 wangchao316 left a comment

Choose a reason for hiding this comment

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

LGTM

@bigreybear bigreybear requested a review from wangchao316 March 30, 2023 13:10
@MarcosZyk MarcosZyk merged commit cf12987 into apache:rel/1.1 Mar 31, 2023
@bigreybear bigreybear deleted the fix_bmec_1.1 branch March 25, 2024 01:44
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