Skip to content

[format] Support size based ORC memory protection for large rows - #9534

Open
ArnavBalyan wants to merge 1 commit into
apache:masterfrom
ArnavBalyan:arnavb/orc-mem
Open

[format] Support size based ORC memory protection for large rows#9534
ArnavBalyan wants to merge 1 commit into
apache:masterfrom
ArnavBalyan:arnavb/orc-mem

Conversation

@ArnavBalyan

Copy link
Copy Markdown
Member

Purpose

  • ORC checks memory growth after a x number of rows. When row size is large, this allowing stripe memory to grow far beyond the configured size before hitting the row count.
  • Add memory based check which can detect memory growth even when the row count is low.
  • This is helpful when rows contain large amount of data like embeddings, vectors etc.

Tests

  • UT

rowsSinceCheck < rowsPerCheck && stripeSizePerCheck == 0
? 0
: treeWriter.estimateMemory();
if (rowsSinceCheck >= rowsPerCheck || size > stripeSizePerCheck) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Honor sub-1 stripe-size ratios

This new gate says a stripe is checked once estimateMemory() exceeds ratio * orc.stripe.size, but the unchanged flush condition below still compares only with memoryLimit (normally the full stripe size). For example, with ratio 0.5 and an unscaled writer, this branch starts firing at half a stripe, resets rowsSinceCheck on every subsequent batch, yet does not flush until the estimate exceeds the full stripe size. That both violates the option description and adds an estimate on every batch in between. Please either flush against the configured threshold (while still respecting a lower MemoryManager limit), or reject/document ratios below 1 if they are not supported.

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.

2 participants