Skip to content

[sql] fix DEFAULT_INITIAL_BUFFER_SIZE#1587

Closed
scwf wants to merge 1 commit intoapache:masterfrom
scwf:fixColumnBuilder
Closed

[sql] fix DEFAULT_INITIAL_BUFFER_SIZE#1587
scwf wants to merge 1 commit intoapache:masterfrom
scwf:fixColumnBuilder

Conversation

@scwf
Copy link
Contributor

@scwf scwf commented Jul 25, 2014

No description provided.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Copy link
Contributor

Choose a reason for hiding this comment

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

While this is definitely a bug fix, it is also pretty dangerous. This means a 100 column table would use 1G of buffer per task, and with 32 cores, that's 32G of ram just for the buffer. @marmbrus @liancheng

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, lets wait to fix this until we fix the other issues....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i am voluntary to fix this issue, can you assign this issue for me?@rxin @marmbrus

@liancheng
Copy link
Contributor

@marmbrus has already filed SPARK-2650 to track this issue and assigned to me. The 104 here is definitely a stupid typo (my bad, sorry...). But the more important missing piece here is that we haven't implemented the logic to calculate a proper initial buffer size like what Shark does. Shark initializes the buffer size by estimating the size of the target column based on DFS block size and ColumnType.defaultSize (see here and here).

@scwf
Copy link
Contributor Author

scwf commented Jul 25, 2014

ok.thanks!

@scwf scwf closed this Jul 25, 2014
@scwf scwf deleted the fixColumnBuilder branch August 22, 2014 15:27
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.

5 participants