-
Notifications
You must be signed in to change notification settings - Fork 151
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
[#2317] feat(server): Introduce server flush benchmark #2318
base: master
Are you sure you want to change the base?
Conversation
Could you use |
@jerqi Sorry, I have no experience about |
It's a little weird to use web page to trigger the benchmark. It maybe dangerous for production environment. |
@jerqi Do you think it would be better if we set a benchmark enable config to control this? It is just a rest API, not an actual web page |
It's also weird to use REST API. |
@jerqi I see, do you have any suggestion? |
You can refer to Iceberg benchmark. https://github.com/apache/iceberg/tree/main/spark/v3.5/spark-extensions/src/jmh/java/org/apache/iceberg |
Nice improvement. BTW, is it possible to introduce the feature of general end to end benchmark test, like using the client to create the fake data to send shuffle-server and then to read? WDYT? @maobaolong |
Is this what you motioned? |
I have seen the feature https://github.com/apache/incubator-uniffle/tree/master/tools/client-simulation-yarn, that is my thought of above mentioned. Thanks again for your effort @maobaolong . For this PR, can you put the bench module into the root tool directory to avoid code pollution. If this, I think the api invoking is acceptable. Because it will only start the bench shuffle-server for flushing benchmark. |
You can use similar method to Alluxio. You use cli to start the benchmark tests. |
What changes were proposed in this pull request?
Introduce server flush benchmark.
Why are the changes needed?
Fix: #2317
Does this PR introduce any user-facing change?
No.
How was this patch tested?