Skip to content

Limit pool size used to process Json/rpc requests#11456

Merged
skabashnyuk merged 12 commits intomasterfrom
i5443
Oct 5, 2018
Merged

Limit pool size used to process Json/rpc requests#11456
skabashnyuk merged 12 commits intomasterfrom
i5443

Conversation

@skabashnyuk
Copy link
Copy Markdown
Contributor

What does this PR do?

Limit pool size used to process Json/rpc requests

What issues does this PR fix or reference?

#5443

Release Notes

n/a

Docs PR

n/a

@benoitf benoitf added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Oct 3, 2018

@Inject
public ServerSideRequestProcessor(
@Named("che.server.jsonrpc.processor_pool_size") String poolSize) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We can inject int instead of String.

@skabashnyuk
Copy link
Copy Markdown
Contributor Author

ci-test

che.server.secure_exposer.jwtproxy.memory_limit=128mb

# Maximum size of the json processing pool
che.server.jsonrpc.processor_max_pool_size=100
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think that this property is related to Che Server. Please consider renaming it to something more common like che.core.jsonrpc... or che.jsonrpc.... . Also, it makes sense to place it near WebSocket related properties.

che.server.secure_exposer.jwtproxy.image=eclipse/che-jwtproxy:latest
che.server.secure_exposer.jwtproxy.memory_limit=128mb

# Maximum size of the json processing pool

This comment was marked as resolved.

private final int maxPoolSize;

@Inject
public ServerSideRequestProcessor(@Named("che.server.jsonrpc.processor_max_pool_size") int maxPoolSize) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

don't forget to rename here as well

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Oct 3, 2018

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11456
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@skabashnyuk
Copy link
Copy Markdown
Contributor Author

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Oct 3, 2018

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11456
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@skabashnyuk
Copy link
Copy Markdown
Contributor Author

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Oct 3, 2018

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11456
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@skabashnyuk skabashnyuk requested a review from tolusha as a code owner October 3, 2018 15:02
@skabashnyuk
Copy link
Copy Markdown
Contributor Author

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Oct 3, 2018

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11456
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@skabashnyuk
Copy link
Copy Markdown
Contributor Author

@eclipse/eclipse-che-qa can you take a look?

@skabashnyuk
Copy link
Copy Markdown
Contributor Author

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Oct 4, 2018

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11456
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@skabashnyuk
Copy link
Copy Markdown
Contributor Author

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Oct 4, 2018

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11456
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@mshaposhnik
Copy link
Copy Markdown
Contributor

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Oct 4, 2018

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11456
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@skabashnyuk
Copy link
Copy Markdown
Contributor Author

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Oct 5, 2018

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11456
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@artaleks9
Copy link
Copy Markdown
Contributor

Selenium tests execution on Eclipse Che Multiuser on OCP (https://ci.codenvycorp.com/job/che-pullrequests-test-ocp/930/) doesn't show any regression against this Pull Request.

@skabashnyuk skabashnyuk merged commit ad8fd5e into master Oct 5, 2018
@skabashnyuk skabashnyuk deleted the i5443 branch October 5, 2018 13:56
nickboldt pushed a commit to nickboldt/che that referenced this pull request Oct 5, 2018
nickboldt pushed a commit to nickboldt/che that referenced this pull request Oct 5, 2018
riuvshin pushed a commit that referenced this pull request Oct 8, 2018
Limit pool size used to process Json/RPC requests (#11456)
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 9, 2018
@benoitf benoitf added this to the 6.13.0 milestone Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Outline of a bug - must adhere to the bug report template.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants