Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

TAJO-2036: Prevent out of memory in the master server, if the query result is large#928

Closed
combineads wants to merge 5 commits intoapache:masterfrom
combineads:TAJO-2036
Closed

TAJO-2036: Prevent out of memory in the master server, if the query result is large#928
combineads wants to merge 5 commits intoapache:masterfrom
combineads:TAJO-2036

Conversation

@combineads
Copy link
Contributor

This problem will occur frequently in our production server.
Please confirm.

Copy link
Contributor

Choose a reason for hiding this comment

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

numOfRows = (long)((float)(resultRows) * ((float)sizeLimit / (float) resultRows)); on the 506 line looks not necessary. Would you remove it, too?

@combineads
Copy link
Contributor Author

@jihoonson Anything else to commit this PR?

@jihoonson
Copy link
Contributor

@combineads, sorry for late response.
Actually, my intention was to keep both factors, but clean some unnecessary codes.
I think we need to keep only the max number of rows rather than max byte size in the future. However, for now, it would be ok if we support both factors. How about adding the max number of rows again?

@combineads
Copy link
Contributor Author

Hi @jihoonson .
I did to add the row limit field.
Thank you for review.

Copy link
Contributor

Choose a reason for hiding this comment

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

limitRow also needs to be added to returnValue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, my mistake. Never mind.

@combineads
Copy link
Contributor Author

Hi @jihoonson .
I did to remove the max row limit.
Thank you for review.

@jihoonson
Copy link
Contributor

+1 LGTM. This patch does not relate to unit tests.
I'll commit shortly.

@asfgit asfgit closed this in e9e7a36 Jan 28, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants