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

[VL] Make velox writer queue size configurable #6341

Merged
merged 1 commit into from
Jul 6, 2024

Conversation

yikf
Copy link
Contributor

@yikf yikf commented Jul 5, 2024

What changes were proposed in this pull request?

This PR aims to make velox writer queue size is configurable to avoid oom from being caused by too many batches.

How was this patch tested?

GA

@yikf
Copy link
Contributor Author

yikf commented Jul 5, 2024

cc @PHILO-HE

Copy link

github-actions bot commented Jul 5, 2024

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

Copy link

github-actions bot commented Jul 5, 2024

Run Gluten Clickhouse CI

@PHILO-HE PHILO-HE changed the title [VELOX] Let velox writer queue is configurable. [VELOX] Make velox writer queue size configurable Jul 5, 2024
@PHILO-HE PHILO-HE changed the title [VELOX] Make velox writer queue size configurable [VL] Make velox writer queue size configurable Jul 5, 2024
@PHILO-HE
Copy link
Contributor

PHILO-HE commented Jul 5, 2024

@yikf, it looks good to me. Did you observe that this config can help relieve the OOM issue on your side?

BTW, please fix code style issue, see reference:
https://github.com/apache/incubator-gluten/blob/main/docs/developers/NewToGluten.md#javascala-code-style

Copy link

github-actions bot commented Jul 5, 2024

Run Gluten Clickhouse CI

@yikf
Copy link
Contributor Author

yikf commented Jul 5, 2024

BTW, please fix code style issue, see reference:
https://github.com/apache/incubator-gluten/blob/main/docs/developers/NewToGluten.md#javascala-code-style

thanks for the reminding of codestyle.

it looks good to me. Did you observe that this config can help relieve the OOM issue on your side?

yes, we have a scenario which trigger an oom is due to a large number of batches being held in a queue. making queue configurable may reduce oom occurrence, but it will also block the query thread, which is not optimal, but it is the fastest way to prevent oom. then we can implement push-based writing to refactor the logic of this piece

@PHILO-HE PHILO-HE requested a review from JkSelf July 5, 2024 06:07
@PHILO-HE PHILO-HE merged commit 7d5ccdb into apache:main Jul 6, 2024
41 checks passed
@yikf yikf deleted the velox-write-queue branch July 6, 2024 16:08
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.

None yet

2 participants